Replace #422: Remove the redundant location setting in AST->SPIR-V.

This was redundant in two ways:
1) it replicated algorithms owned in the front end, and
2) it sometimes left location information on both a block and its members.

diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp
index e175bfc6516f055ac1080063047d9dce55588e17..bc483c7e6d092124b6745bbaba1d8525096a603c 100755 (executable)
--- a/SPIRV/GlslangToSpv.cpp
+++ b/SPIRV/GlslangToSpv.cpp
@@ -2407,7 +2407,6 @@ spv::Id TGlslangToSpvTraverser::convertGlslangStructToSpvType(const glslang::TTy
     // Create a vector of struct types for SPIR-V to consume
     std::vector<spv::Id> spvMembers;
     int memberDelta = 0;  // how much the member's index changes from glslang to SPIR-V, normally 0, except sometimes for blocks
-    int locationOffset = 0;  // for use across struct members, when they are called recursively
     for (int i = 0; i < (int)glslangMembers->size(); i++) {
         glslang::TType& glslangMember = *(*glslangMembers)[i].type;
         if (glslangMember.hiddenMember()) {
@@ -2424,11 +2423,9 @@ spv::Id TGlslangToSpvTraverser::convertGlslangStructToSpvType(const glslang::TTy
             glslang::TQualifier memberQualifier = glslangMember.getQualifier();
             InheritQualifiers(memberQualifier, qualifier);
 
-            // manually inherit location; it's more complex
+            // manually inherit location
             if (! memberQualifier.hasLocation() && qualifier.hasLocation())
-                memberQualifier.layoutLocation = qualifier.layoutLocation + locationOffset;
-            if (qualifier.hasLocation())
-                locationOffset += glslangIntermediate->computeTypeLocationSize(glslangMember);
+                memberQualifier.layoutLocation = qualifier.layoutLocation;
 
             // recurse
             spvMembers.push_back(convertGlslangToSpvType(glslangMember, explicitLayout, memberQualifier));
@@ -2491,29 +2488,12 @@ void TGlslangToSpvTraverser::decorateStructType(const glslang::TType& type,
                     addMemberDecoration(spvType, member, memory[i]);
             }
 
-            // 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
+            // Location assignment was already completed correctly by the front end,
+            // just track whether a member needs to be decorated.
             // 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);
+            // ill-specified and decisions have been made to not allow this.
+            if (! type.isArray() && memberQualifier.hasLocation())
+                builder.addMemberDecoration(spvType, member, spv::DecorationLocation, memberQualifier.layoutLocation);
 
             if (qualifier.hasLocation())      // track for upcoming inheritance
                 locationOffset += glslangIntermediate->computeTypeLocationSize(glslangMember);

diff --git a/Test/baseResults/spv.450.tesc.out b/Test/baseResults/spv.450.tesc.out
index 6244be12d8853344690dd5815c01474e001904f1..38d38c5600b3f9eb8af0e6f1c0e589e93a62fbe6 100755 (executable)
--- a/Test/baseResults/spv.450.tesc.out
+++ b/Test/baseResults/spv.450.tesc.out
@@ -38,11 +38,8 @@ Warning, version 450 is not yet complete; most version-specific features are pre
                               Decorate 11(TheBlock) Block
                               Decorate 16(tcBlock) Location 12
                               MemberDecorate 17(SingleBlock) 0 Patch
-                              MemberDecorate 17(SingleBlock) 0 Location 2
                               MemberDecorate 17(SingleBlock) 1 Patch
-                              MemberDecorate 17(SingleBlock) 1 Location 3
                               MemberDecorate 17(SingleBlock) 2 Patch
-                              MemberDecorate 17(SingleBlock) 2 Location 4
                               Decorate 17(SingleBlock) Block
                               Decorate 19(singleBlock) Location 2
                               MemberDecorate 20(bn) 0 Patch