KhronosGroup / KhronosGroup/glslang

HLSL incorrect global constant order results in broken SPIR-V

Open
#2,047 0 comments 0 reactions 0 assignees View on GitHub
HLSL
Dominant language
C++
Stars
3.6k
Forks
989
Avg merge
1d 2h
Merged PRs (30d)
31

Description

When using hlsl and specifying global constants as uniform float4 const : register(cXXX) those constants are assembled in $Global cb in order they appear in source and not in order specified by corresponding cXXX register. This results in offsets not being sequentially growing in spir-v

```
uniform float4 const1 : register(c0);
uniform float4 const3 : register(c10);
uniform float4 const2 : register(c2);

float4 main() : SV_Position
{
return float4(const1.x, const2.x, const3.x, 1.0);
}
```

```
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Vertex %main "main" %_entryPointOutput
OpSource HLSL 500
OpName %main "main"
OpName %_main_ "@main("
OpName %_Global "$Global"
OpMemberName %_Global 0 "const1"
OpMemberName %_Global 1 "const3"
OpMemberName %_Global 2 "const2"
OpName %_ ""
OpName %_entryPointOutput "@entryPointOutput"
OpMemberDecorate %_Global 0 Offset 0
OpMemberDecorate %_Global 1 Offset 160
OpMemberDecorate %_Global 2 Offset 32
OpDecorate %_Global Block
OpDecorate %_ DescriptorSet 4
OpDecorate %_ Binding 0
OpDecorate %_entryPointOutput BuiltIn Position
%void = OpTypeVoid
%3 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%8 = OpTypeFunction %v4float
%_Global = OpTypeStruct %v4float %v4float %v4float
%_ptr_Uniform__Global = OpTypePointer Uniform %_Global
%_ = OpVariable %_ptr_Uniform__Global Uniform
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%uint = OpTypeInt 32 0
%uint_0 = OpConstant %uint 0
%_ptr_Uniform_float = OpTypePointer Uniform %float
%int_2 = OpConstant %int 2
%int_1 = OpConstant %int 1
%float_1 = OpConstant %float 1
%_ptr_Output_v4float = OpTypePointer Output %v4float
%_entryPointOutput = OpVariable %_ptr_Output_v4float Output
%main = OpFunction %void None %3
%5 = OpLabel
%33 = OpFunctionCall %v4float %_main_
OpStore %_entryPointOutput %33
OpReturn
OpFunctionEnd
%_main_ = OpFunction %v4float None %8
%10 = OpLabel
%19 = OpAccessChain %_ptr_Uniform_float %_ %int_0 %uint_0
%20 = OpLoad %float %19
%22 = OpAccessChain %_ptr_Uniform_float %_ %int_2 %uint_0
%23 = OpLoad %float %22
%25 = OpAccessChain %_ptr_Uniform_float %_ %int_1 %uint_0
%26 = OpLoad %float %25
%28 = OpCompositeConstruct %v4float %20 %23 %26 %float_1
OpReturnValue %28
OpFunctionEnd
```

here's a workaround we came up with

```
diff --git a/glslang/MachineIndependent/ParseContextBase.cpp b/glslang/MachineIndependent/ParseContextBase.cpp
index 282ecca0e07..3855856cca1 100644
--- a/glslang/MachineIndependent/ParseContextBase.cpp
+++ b/glslang/MachineIndependent/ParseContextBase.cpp
@@ -605,7 +605,13 @@ void TParseContextBase::growGlobalUniformBlock(const TSourceLoc& loc, TType& mem
if (typeList)
type->setStruct(typeList);
TTypeLoc typeLoc = {type, loc};
- globalUniformBlock->getType().getWritableStruct()->push_back(typeLoc);
+ auto & struc = * globalUniformBlock->getType().getWritableStruct();
+ struc.push_back(typeLoc);
+
+ std::sort(struc.begin(), struc.end(), [](const auto& a, const auto& b)
+ {
+ return a.type->getQualifier().layoutOffset < b.type->getQualifier().layoutOffset;
+ });

// Insert into the symbol table.
if (firstNewMember == 0) {
@@ -616,7 +622,7 @@ void TParseContextBase::growGlobalUniformBlock(const TSourceLoc& loc, TType& mem
error(loc, "failed to insert the global constant buffer", "uniform", "");
} else {
// This is a follow-on request; we need to amend the first insert
- symbolTable.amend(*globalUniformBlock, firstNewMember);
+ symbolTable.amend(*globalUniformBlock, 0);
}

++firstNewMember;
diff --git a/glslang/MachineIndependent/SymbolTable.h b/glslang/MachineIndependent/SymbolTable.h
index 40ca3da532c..892e1196a87 100644
--- a/glslang/MachineIndependent/SymbolTable.h
+++ b/glslang/MachineIndependent/SymbolTable.h
@@ -359,6 +359,7 @@ public:
virtual const TAnonMember* getAsAnonMember() const override { return this; }
virtual const TVariable& getAnonContainer() const { return anonContainer; }
virtual unsigned int getMemberNumber() const { return memberNumber; }
+ void setMemberNumber(unsigned int m) { memberNumber = m; }

virtual const TType& getType() const override
{
@@ -449,10 +450,19 @@ public:
bool insertAnonymousMembers(TSymbol& symbol, int firstMember)
{
const TTypeList& types = *symbol.getAsVariable()->getType().getStruct();
- for (unsigned int m = firstMember; m < types.size(); ++m) {
- TAnonMember* member = new TAnonMember(&types[m].type->getFieldName(), m, *symbol.getAsVariable(), symbol.getAsVariable()->getAnonId());
- if (! level.insert(tLevelPair(member->getMangledName(), member)).second)
- return false;
+ for (unsigned int m = firstMember; m < types.size(); ++m)
+ {
+ if (level.find(types[m].type->getFieldName()) != level.end())
+ {
+ TAnonMember* member = (TAnonMember*)level[types[m].type->getFieldName()];
+ member->setMemberNumber(m);
+ }
+ else
+ {
+ TAnonMember* member = new TAnonMember(&types[m].type->getFieldName(), m, *symbol.getAsVariable(), symbol.getAsVariable()->getAnonId());
+ if (!level.insert(tLevelPair(member->getMangledName(), member)).second)
+ return false;
+ }
}

return true;

```

Contributor guide

Open the contributing guide

Research direction

Start in glslang/MachineIndependent/ParseContextBase.cpp at TParseContextBase::growGlobalUniformBlock and inspect related member handling in glslang/MachineIndependent/SymbolTable.h. Reproduce the HLSL example and verify that generated SPIR-V orders $Global members by cXXX register, with sequential offsets and correct accesses; the workaround provides expected behavior to compare against.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.