From: John Kessenich Date: Fri, 1 Jul 2016 03:47:35 +0000 (-0600) Subject: SPV: Don't decorate locations within an array, it doesn't make sense. X-Git-Tag: upstream/0.1~105 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2f47bc978145b1afa1261b6004a18113da1780ac;p=platform%2Fupstream%2Fglslang.git SPV: Don't decorate locations within an array, it doesn't make sense. This fixes issue #360. --- diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index 6c9068f..de87763 100755 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -2019,25 +2019,33 @@ void TGlslangToSpvTraverser::decorateStructType(const glslang::TType& type, addMemberDecoration(spvType, member, memory[i]); } - // compute location decoration; tricky based on whether inheritance is at play + // Compute location decoration; tricky based on whether inheritance is at play and + // what kind of container we have, etc. // TODO: This algorithm (and it's cousin above doing almost the same thing) should // probably move to the linker stage of the front end proper, and just have the // answer sitting already distributed throughout the individual member locations. int location = -1; // will only decorate if present or inherited - if (memberQualifier.hasLocation()) { // no inheritance, or override of inheritance - // struct members should not have explicit locations - assert(type.getBasicType() != glslang::EbtStruct); - location = memberQualifier.layoutLocation; - } else if (type.getBasicType() != glslang::EbtBlock) { - // If it is a not a Block, (...) Its members are assigned consecutive locations (...) - // The members, and their nested types, must not themselves have Location decorations. - } else if (qualifier.hasLocation()) // inheritance - location = qualifier.layoutLocation + locationOffset; - if (qualifier.hasLocation()) // track for upcoming inheritance - locationOffset += glslangIntermediate->computeTypeLocationSize(glslangMember); + // Ignore member locations if the container is an array, as that's + // ill-specified and decisions have been made to not allow this anyway. + // The object itself must have a location, and that comes out from decorating the object, + // not the type (this code decorates types). + if (! type.isArray()) { + if (memberQualifier.hasLocation()) { // no inheritance, or override of inheritance + // struct members should not have explicit locations + assert(type.getBasicType() != glslang::EbtStruct); + location = memberQualifier.layoutLocation; + } else if (type.getBasicType() != glslang::EbtBlock) { + // If it is a not a Block, (...) Its members are assigned consecutive locations (...) + // The members, and their nested types, must not themselves have Location decorations. + } else if (qualifier.hasLocation()) // inheritance + location = qualifier.layoutLocation + locationOffset; + } if (location >= 0) builder.addMemberDecoration(spvType, member, spv::DecorationLocation, location); + if (qualifier.hasLocation()) // track for upcoming inheritance + locationOffset += glslangIntermediate->computeTypeLocationSize(glslangMember); + // component, XFB, others if (glslangMember.getQualifier().hasComponent()) builder.addMemberDecoration(spvType, member, spv::DecorationComponent, glslangMember.getQualifier().layoutComponent); diff --git a/Test/baseResults/spv.450.tesc.out b/Test/baseResults/spv.450.tesc.out new file mode 100755 index 0000000..c6c08b2 --- /dev/null +++ b/Test/baseResults/spv.450.tesc.out @@ -0,0 +1,50 @@ +spv.450.tesc +Warning, version 450 is not yet complete; most version-specific features are present, but some are missing. + + +Linked tessellation control stage: + + +// Module Version 10000 +// Generated by (magic number): 80001 +// Id's are bound by 17 + + Capability Tessellation + 1: ExtInstImport "GLSL.std.450" + MemoryModel Logical GLSL450 + EntryPoint TessellationControl 4 "main" 9 16 + ExecutionMode 4 OutputVertices 4 + Source GLSL 450 + Name 4 "main" + Name 9 "patchOut" + Name 10 "S" + MemberName 10(S) 0 "sMem1" + MemberName 10(S) 1 "sMem2" + Name 11 "TheBlock" + MemberName 11(TheBlock) 0 "bMem1" + MemberName 11(TheBlock) 1 "bMem2" + MemberName 11(TheBlock) 2 "s" + Name 16 "tcBlock" + Decorate 9(patchOut) Patch + MemberDecorate 11(TheBlock) 0 Patch + MemberDecorate 11(TheBlock) 1 Patch + MemberDecorate 11(TheBlock) 2 Patch + Decorate 11(TheBlock) Block + Decorate 16(tcBlock) Location 12 + 2: TypeVoid + 3: TypeFunction 2 + 6: TypeFloat 32 + 7: TypeVector 6(float) 4 + 8: TypePointer Output 7(fvec4) + 9(patchOut): 8(ptr) Variable Output + 10(S): TypeStruct 6(float) 6(float) + 11(TheBlock): TypeStruct 6(float) 6(float) 10(S) + 12: TypeInt 32 0 + 13: 12(int) Constant 2 + 14: TypeArray 11(TheBlock) 13 + 15: TypePointer Output 14 + 16(tcBlock): 15(ptr) Variable Output + 4(main): 2 Function None 3 + 5: Label + Return + FunctionEnd diff --git a/Test/spv.450.tesc b/Test/spv.450.tesc new file mode 100644 index 0000000..0f8ec5c --- /dev/null +++ b/Test/spv.450.tesc @@ -0,0 +1,20 @@ +#version 450 core + +layout(vertices = 4) out; + +patch out vec4 patchOut; + +struct S { + float sMem1; // should not see a patch decoration + float sMem2; // should not see a patch decoration +}; + +layout(location = 12) patch out TheBlock { + highp float bMem1; // should not see a location decoration + highp float bMem2; + S s; // should see a patch decoration +} tcBlock[2]; + +void main() +{ +} diff --git a/gtests/Spv.FromFile.cpp b/gtests/Spv.FromFile.cpp index 3906779..04787ea 100644 --- a/gtests/Spv.FromFile.cpp +++ b/gtests/Spv.FromFile.cpp @@ -107,6 +107,7 @@ INSTANTIATE_TEST_CASE_P( "spv.400.tese", "spv.420.geom", "spv.430.vert", + "spv.450.tesc", "spv.accessChain.frag", "spv.aggOps.frag", "spv.always-discard.frag",