From 7cdf3fc3c61f3c62bfc96aa471082fa4c38f4cc3 Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Sun, 4 Jun 2017 13:22:39 -0600 Subject: [PATCH] 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. --- SPIRV/GlslangToSpv.cpp | 34 +++++++------------------------ Test/baseResults/spv.450.tesc.out | 3 --- 2 files changed, 7 insertions(+), 30 deletions(-) diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index e175bfc6..bc483c7e 100755 --- 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 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 6244be12..38d38c56 100755 --- 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