SPV: Don't decorate locations within an array, it doesn't make sense.
authorJohn Kessenich <cepheus@frii.com>
Fri, 1 Jul 2016 03:47:35 +0000 (21:47 -0600)
committerJohn Kessenich <cepheus@frii.com>
Fri, 1 Jul 2016 04:00:09 +0000 (22:00 -0600)
This fixes issue #360.

SPIRV/GlslangToSpv.cpp
Test/baseResults/spv.450.tesc.out [new file with mode: 0755]
Test/spv.450.tesc [new file with mode: 0644]
gtests/Spv.FromFile.cpp

index 6c9068f..de87763 100755 (executable)
@@ -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 (executable)
index 0000000..c6c08b2
--- /dev/null
@@ -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 (file)
index 0000000..0f8ec5c
--- /dev/null
@@ -0,0 +1,20 @@
+#version 450 core\r
+\r
+layout(vertices = 4) out;\r
+\r
+patch out vec4 patchOut;\r
+\r
+struct S {\r
+    float sMem1;  // should not see a patch decoration\r
+    float sMem2;  // should not see a patch decoration\r
+};\r
+\r
+layout(location = 12) patch out TheBlock {\r
+    highp float bMem1;  // should not see a location decoration\r
+    highp float bMem2;\r
+    S s;                // should see a patch decoration\r
+} tcBlock[2];\r
+\r
+void main()\r
+{\r
+}\r
index 3906779..04787ea 100644 (file)
@@ -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",