From 0967748fbce0773625dcba0f1185e1dd79092c0d Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Fri, 19 Feb 2016 12:21:50 -0700 Subject: [PATCH] SPV: Fix 'location' inheritance bug. --- SPIRV/GlslangToSpv.cpp | 30 ++++++++++++++++++++++-------- Test/baseResults/spv.430.vert.out | 16 ++++++++++++---- Test/spv.430.vert | 3 ++- glslang/Include/BaseTypes.h | 2 +- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index 864902ccb..21a04b0ec 100755 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -535,8 +535,6 @@ void InheritQualifiers(glslang::TQualifier& child, const glslang::TQualifier& pa child.patch = true; if (parent.sample) child.sample = true; - - child.layoutLocation = parent.layoutLocation; } bool HasNonLayoutQualifiers(const glslang::TQualifier& qualifier) @@ -1723,10 +1721,14 @@ spv::Id TGlslangToSpvTraverser::convertGlslangToSpvType(const glslang::TType& ty // modify just this child's view of the qualifier glslang::TQualifier subQualifier = glslangType.getQualifier(); InheritQualifiers(subQualifier, qualifier); - if (qualifier.hasLocation()) { - subQualifier.layoutLocation += locationOffset; + + // manually inherit location; it's more complex + if (! subQualifier.hasLocation() && qualifier.hasLocation()) + subQualifier.layoutLocation = qualifier.layoutLocation + locationOffset; + if (qualifier.hasLocation()) locationOffset += glslangIntermediate->computeTypeLocationSize(glslangType); - } + + // recurse structFields.push_back(convertGlslangToSpvType(glslangType, explicitLayout, subQualifier)); } } @@ -1756,10 +1758,22 @@ spv::Id TGlslangToSpvTraverser::convertGlslangToSpvType(const glslang::TType& ty addMemberDecoration(spvType, member, TranslatePrecisionDecoration(glslangType)); addMemberDecoration(spvType, member, TranslateInterpolationDecoration(subQualifier)); addMemberDecoration(spvType, member, TranslateInvariantDecoration(subQualifier)); - if (qualifier.hasLocation()) { - builder.addMemberDecoration(spvType, member, spv::DecorationLocation, qualifier.layoutLocation + locationOffset); + + // compute location decoration; tricky based on whether inheritance is at play + // 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 (subQualifier.hasLocation()) // no inheritance, or override of inheritance + location = subQualifier.layoutLocation; + else if (qualifier.hasLocation()) // inheritance + location = qualifier.layoutLocation + locationOffset; + if (qualifier.hasLocation()) // track for upcoming inheritance locationOffset += glslangIntermediate->computeTypeLocationSize(glslangType); - } + if (location >= 0) + builder.addMemberDecoration(spvType, member, spv::DecorationLocation, location); + + // component, XFB, others if (glslangType.getQualifier().hasComponent()) builder.addMemberDecoration(spvType, member, spv::DecorationComponent, glslangType.getQualifier().layoutComponent); if (glslangType.getQualifier().hasXfbOffset()) diff --git a/Test/baseResults/spv.430.vert.out b/Test/baseResults/spv.430.vert.out index a851b1c41..a1487d53f 100755 --- a/Test/baseResults/spv.430.vert.out +++ b/Test/baseResults/spv.430.vert.out @@ -1,5 +1,5 @@ spv.430.vert -Warning, version 430 is not yet complete; most version-specific features are present, but some are missing. +Warning, version 450 is not yet complete; most version-specific features are present, but some are missing. Linked vertex stage: @@ -7,14 +7,14 @@ Linked vertex stage: // Module Version 10000 // Generated by (magic number): 80001 -// Id's are bound by 63 +// Id's are bound by 66 Capability Shader Capability ClipDistance 1: ExtInstImport "GLSL.std.450" MemoryModel Logical GLSL450 - EntryPoint Vertex 4 "main" 12 23 34 38 41 42 62 - Source GLSL 430 + EntryPoint Vertex 4 "main" 12 23 34 38 41 42 62 65 + Source GLSL 450 Name 4 "main" Name 10 "gl_PerVertex" MemberName 10(gl_PerVertex) 0 "gl_ClipDistance" @@ -42,6 +42,9 @@ Linked vertex stage: MemberName 60(SS) 1 "s" MemberName 60(SS) 2 "c" Name 62 "var" + Name 63 "MS" + MemberName 63(MS) 0 "f" + Name 65 "outMS" MemberDecorate 10(gl_PerVertex) 0 BuiltIn ClipDistance Decorate 10(gl_PerVertex) Block Decorate 34(badorder3) Flat @@ -72,6 +75,8 @@ Linked vertex stage: MemberDecorate 60(SS) 1 Location 1 MemberDecorate 60(SS) 2 Flat MemberDecorate 60(SS) 2 Location 4 + MemberDecorate 63(MS) 0 Location 17 + Decorate 63(MS) Block 2: TypeVoid 3: TypeFunction 2 6: TypeFloat 32 @@ -121,6 +126,9 @@ Linked vertex stage: 60(SS): TypeStruct 19(fvec4) 59(S) 19(fvec4) 61: TypePointer Output 60(SS) 62(var): 61(ptr) Variable Output + 63(MS): TypeStruct 6(float) + 64: TypePointer Output 63(MS) + 65(outMS): 64(ptr) Variable Output 4(main): 2 Function None 3 5: Label 18: 17(ptr) AccessChain 12 14 15 diff --git a/Test/spv.430.vert b/Test/spv.430.vert index d0b05ecde..f52ff4a64 100644 --- a/Test/spv.430.vert +++ b/Test/spv.430.vert @@ -1,4 +1,4 @@ -#version 430 core +#version 450 core @@ -34,3 +34,4 @@ layout(binding = 31) uniform sampler2D sampb4; struct S { mediump float a; highp uvec2 b; highp vec3 c; }; struct SS { vec4 b; S s; vec4 c; }; layout(location = 0) flat out SS var; +out MS { layout(location = 17) float f; } outMS; diff --git a/glslang/Include/BaseTypes.h b/glslang/Include/BaseTypes.h index ca1fa4c3b..5a57664fb 100644 --- a/glslang/Include/BaseTypes.h +++ b/glslang/Include/BaseTypes.h @@ -71,7 +71,7 @@ enum TStorageQualifier { EvqGlobal, // For globals read/write EvqConst, // User-defined constant values, will be semantically constant and constant folded EvqVaryingIn, // pipeline input, read only, also supercategory for all built-ins not included in this enum (see TBuiltInVariable) - EvqVaryingOut, // pipeline ouput, read/write, also supercategory for all built-ins not included in this enum (see TBuiltInVariable) + EvqVaryingOut, // pipeline output, read/write, also supercategory for all built-ins not included in this enum (see TBuiltInVariable) EvqUniform, // read only, shared with app EvqBuffer, // read/write, shared with app EvqShared, // compute shader's read/write 'shared' qualifier