KhronosGroup / KhronosGroup/glslang
HLSL incorrect global constant order results in broken SPIR-V
- 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
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