Fix issue with remapping global uniform blocks
authorMalcolm Bechard <malcolm@derivative.ca>
Wed, 17 Mar 2021 22:47:13 +0000 (18:47 -0400)
committerMalcolm Bechard <malcolm@derivative.ca>
Wed, 17 Mar 2021 23:30:22 +0000 (19:30 -0400)
Avoid adding global uniform blocks to stages that don't already have it.
Otherwise multiple stages point to the same block object, and a
remapping that occurs later on will change the mapping on multiple
stages.

Test/baseResults/vk.relaxed.changeSet.vert.out [new file with mode: 0755]
Test/vk.relaxed.changeSet.frag [new file with mode: 0755]
Test/vk.relaxed.changeSet.vert [new file with mode: 0755]
glslang/MachineIndependent/ShaderLang.cpp
glslang/MachineIndependent/linkValidate.cpp
glslang/MachineIndependent/localintermediate.h
gtests/VkRelaxed.FromFile.cpp

diff --git a/Test/baseResults/vk.relaxed.changeSet.vert.out b/Test/baseResults/vk.relaxed.changeSet.vert.out
new file mode 100755 (executable)
index 0000000..f6bce29
--- /dev/null
@@ -0,0 +1,281 @@
+vk.relaxed.changeSet.vert
+Shader version: 460
+0:? Sequence
+0:11  Function Definition: main( ( global void)
+0:11    Function Parameters: 
+0:13    Sequence
+0:13      move second child to first child ( temp highp 4-component vector of float)
+0:13        'Color' ( smooth out highp 4-component vector of float)
+0:13        'aColor' ( in highp 4-component vector of float)
+0:14      move second child to first child ( temp highp 2-component vector of float)
+0:14        'UV' ( smooth out highp 2-component vector of float)
+0:14        'aUV' ( in highp 2-component vector of float)
+0:15      move second child to first child ( temp highp 4-component vector of float)
+0:15        gl_Position: direct index for structure ( gl_Position highp 4-component vector of float Position)
+0:15          'anon@1' ( out block{ gl_Position 4-component vector of float Position gl_Position,  gl_PointSize float PointSize gl_PointSize,  out unsized 1-element array of float ClipDistance gl_ClipDistance,  out unsized 1-element array of float CullDistance gl_CullDistance})
+0:15          Constant:
+0:15            0 (const uint)
+0:15        matrix-times-vector ( temp highp 4-component vector of float)
+0:15          projectionMatrix: direct index for structure ( uniform highp 4X4 matrix of float)
+0:15            'anon@0' (layout( column_major std140) uniform block{ uniform highp 4X4 matrix of float projectionMatrix})
+0:15            Constant:
+0:15              0 (const uint)
+0:15          Construct vec4 ( temp highp 4-component vector of float)
+0:15            'aPos' ( in highp 2-component vector of float)
+0:15            Constant:
+0:15              0.000000
+0:15            Constant:
+0:15              1.000000
+0:?   Linker Objects
+0:?     'aPos' ( in highp 2-component vector of float)
+0:?     'aUV' ( in highp 2-component vector of float)
+0:?     'aColor' ( in highp 4-component vector of float)
+0:?     'anon@0' (layout( column_major std140) uniform block{ uniform highp 4X4 matrix of float projectionMatrix})
+0:?     'Color' ( smooth out highp 4-component vector of float)
+0:?     'UV' ( smooth out highp 2-component vector of float)
+0:?     'anon@1' ( out block{ gl_Position 4-component vector of float Position gl_Position,  gl_PointSize float PointSize gl_PointSize,  out unsized 1-element array of float ClipDistance gl_ClipDistance,  out unsized 1-element array of float CullDistance gl_CullDistance})
+0:?     'gl_VertexID' ( in int VertexIndex)
+0:?     'gl_InstanceID' ( in int InstanceIndex)
+
+vk.relaxed.changeSet.frag
+Shader version: 460
+gl_FragCoord origin is upper left
+0:? Sequence
+0:10  Function Definition: main( ( global void)
+0:10    Function Parameters: 
+0:12    Sequence
+0:12      move second child to first child ( temp highp 4-component vector of float)
+0:12        'fragColor' (layout( location=0) out highp 4-component vector of float)
+0:12        vector-scale ( temp highp 4-component vector of float)
+0:12          'Color' ( smooth in highp 4-component vector of float)
+0:12          direct index ( temp highp float)
+0:12            texture ( global highp 4-component vector of float)
+0:12              'sTexture' ( uniform highp sampler2D)
+0:12              vector swizzle ( temp highp 2-component vector of float)
+0:12                'UV' ( smooth in highp 2-component vector of float)
+0:12                Sequence
+0:12                  Constant:
+0:12                    0 (const int)
+0:12                  Constant:
+0:12                    1 (const int)
+0:12            Constant:
+0:12              0 (const int)
+0:?   Linker Objects
+0:?     'fragColor' (layout( location=0) out highp 4-component vector of float)
+0:?     'sTexture' ( uniform highp sampler2D)
+0:?     'Color' ( smooth in highp 4-component vector of float)
+0:?     'UV' ( smooth in highp 2-component vector of float)
+
+
+Linked vertex stage:
+
+
+Linked fragment stage:
+
+
+Shader version: 460
+0:? Sequence
+0:11  Function Definition: main( ( global void)
+0:11    Function Parameters: 
+0:13    Sequence
+0:13      move second child to first child ( temp highp 4-component vector of float)
+0:13        'Color' ( smooth out highp 4-component vector of float)
+0:13        'aColor' ( in highp 4-component vector of float)
+0:14      move second child to first child ( temp highp 2-component vector of float)
+0:14        'UV' ( smooth out highp 2-component vector of float)
+0:14        'aUV' ( in highp 2-component vector of float)
+0:15      move second child to first child ( temp highp 4-component vector of float)
+0:15        gl_Position: direct index for structure ( gl_Position highp 4-component vector of float Position)
+0:15          'anon@1' ( out block{ gl_Position 4-component vector of float Position gl_Position,  gl_PointSize float PointSize gl_PointSize,  out 1-element array of float ClipDistance gl_ClipDistance,  out 1-element array of float CullDistance gl_CullDistance})
+0:15          Constant:
+0:15            0 (const uint)
+0:15        matrix-times-vector ( temp highp 4-component vector of float)
+0:15          projectionMatrix: direct index for structure ( uniform highp 4X4 matrix of float)
+0:15            'anon@0' (layout( column_major std140) uniform block{ uniform highp 4X4 matrix of float projectionMatrix})
+0:15            Constant:
+0:15              0 (const uint)
+0:15          Construct vec4 ( temp highp 4-component vector of float)
+0:15            'aPos' ( in highp 2-component vector of float)
+0:15            Constant:
+0:15              0.000000
+0:15            Constant:
+0:15              1.000000
+0:?   Linker Objects
+0:?     'aPos' ( in highp 2-component vector of float)
+0:?     'aUV' ( in highp 2-component vector of float)
+0:?     'aColor' ( in highp 4-component vector of float)
+0:?     'anon@0' (layout( column_major std140) uniform block{ uniform highp 4X4 matrix of float projectionMatrix})
+0:?     'Color' ( smooth out highp 4-component vector of float)
+0:?     'UV' ( smooth out highp 2-component vector of float)
+0:?     'anon@1' ( out block{ gl_Position 4-component vector of float Position gl_Position,  gl_PointSize float PointSize gl_PointSize,  out 1-element array of float ClipDistance gl_ClipDistance,  out 1-element array of float CullDistance gl_CullDistance})
+0:?     'gl_VertexID' ( in int VertexIndex)
+0:?     'gl_InstanceID' ( in int InstanceIndex)
+Shader version: 460
+gl_FragCoord origin is upper left
+0:? Sequence
+0:10  Function Definition: main( ( global void)
+0:10    Function Parameters: 
+0:12    Sequence
+0:12      move second child to first child ( temp highp 4-component vector of float)
+0:12        'fragColor' (layout( location=0) out highp 4-component vector of float)
+0:12        vector-scale ( temp highp 4-component vector of float)
+0:12          'Color' ( smooth in highp 4-component vector of float)
+0:12          direct index ( temp highp float)
+0:12            texture ( global highp 4-component vector of float)
+0:12              'sTexture' ( uniform highp sampler2D)
+0:12              vector swizzle ( temp highp 2-component vector of float)
+0:12                'UV' ( smooth in highp 2-component vector of float)
+0:12                Sequence
+0:12                  Constant:
+0:12                    0 (const int)
+0:12                  Constant:
+0:12                    1 (const int)
+0:12            Constant:
+0:12              0 (const int)
+0:?   Linker Objects
+0:?     'fragColor' (layout( location=0) out highp 4-component vector of float)
+0:?     'sTexture' ( uniform highp sampler2D)
+0:?     'Color' ( smooth in highp 4-component vector of float)
+0:?     'UV' ( smooth in highp 2-component vector of float)
+
+// Module Version 10000
+// Generated by (magic number): 8000a
+// Id's are bound by 46
+
+                              Capability Shader
+               1:             ExtInstImport  "GLSL.std.450"
+                              MemoryModel Logical GLSL450
+                              EntryPoint Vertex 4  "main" 9 11 15 17 24 34 44 45
+                              Source GLSL 460
+                              Name 4  "main"
+                              Name 9  "Color"
+                              Name 11  "aColor"
+                              Name 15  "UV"
+                              Name 17  "aUV"
+                              Name 22  "gl_PerVertex"
+                              MemberName 22(gl_PerVertex) 0  "gl_Position"
+                              MemberName 22(gl_PerVertex) 1  "gl_PointSize"
+                              MemberName 22(gl_PerVertex) 2  "gl_ClipDistance"
+                              MemberName 22(gl_PerVertex) 3  "gl_CullDistance"
+                              Name 24  ""
+                              Name 28  "gl_DefaultUniformBlock"
+                              MemberName 28(gl_DefaultUniformBlock) 0  "projectionMatrix"
+                              Name 30  ""
+                              Name 34  "aPos"
+                              Name 44  "gl_VertexID"
+                              Name 45  "gl_InstanceID"
+                              Decorate 9(Color) Location 0
+                              Decorate 11(aColor) Location 2
+                              Decorate 15(UV) Location 1
+                              Decorate 17(aUV) Location 1
+                              MemberDecorate 22(gl_PerVertex) 0 BuiltIn Position
+                              MemberDecorate 22(gl_PerVertex) 1 BuiltIn PointSize
+                              MemberDecorate 22(gl_PerVertex) 2 BuiltIn ClipDistance
+                              MemberDecorate 22(gl_PerVertex) 3 BuiltIn CullDistance
+                              Decorate 22(gl_PerVertex) Block
+                              MemberDecorate 28(gl_DefaultUniformBlock) 0 ColMajor
+                              MemberDecorate 28(gl_DefaultUniformBlock) 0 Offset 0
+                              MemberDecorate 28(gl_DefaultUniformBlock) 0 MatrixStride 16
+                              Decorate 28(gl_DefaultUniformBlock) Block
+                              Decorate 30 DescriptorSet 0
+                              Decorate 30 Binding 0
+                              Decorate 34(aPos) Location 0
+                              Decorate 44(gl_VertexID) BuiltIn VertexIndex
+                              Decorate 45(gl_InstanceID) BuiltIn InstanceIndex
+               2:             TypeVoid
+               3:             TypeFunction 2
+               6:             TypeFloat 32
+               7:             TypeVector 6(float) 4
+               8:             TypePointer Output 7(fvec4)
+        9(Color):      8(ptr) Variable Output
+              10:             TypePointer Input 7(fvec4)
+      11(aColor):     10(ptr) Variable Input
+              13:             TypeVector 6(float) 2
+              14:             TypePointer Output 13(fvec2)
+          15(UV):     14(ptr) Variable Output
+              16:             TypePointer Input 13(fvec2)
+         17(aUV):     16(ptr) Variable Input
+              19:             TypeInt 32 0
+              20:     19(int) Constant 1
+              21:             TypeArray 6(float) 20
+22(gl_PerVertex):             TypeStruct 7(fvec4) 6(float) 21 21
+              23:             TypePointer Output 22(gl_PerVertex)
+              24:     23(ptr) Variable Output
+              25:             TypeInt 32 1
+              26:     25(int) Constant 0
+              27:             TypeMatrix 7(fvec4) 4
+28(gl_DefaultUniformBlock):             TypeStruct 27
+              29:             TypePointer Uniform 28(gl_DefaultUniformBlock)
+              30:     29(ptr) Variable Uniform
+              31:             TypePointer Uniform 27
+        34(aPos):     16(ptr) Variable Input
+              36:    6(float) Constant 0
+              37:    6(float) Constant 1065353216
+              43:             TypePointer Input 25(int)
+ 44(gl_VertexID):     43(ptr) Variable Input
+45(gl_InstanceID):     43(ptr) Variable Input
+         4(main):           2 Function None 3
+               5:             Label
+              12:    7(fvec4) Load 11(aColor)
+                              Store 9(Color) 12
+              18:   13(fvec2) Load 17(aUV)
+                              Store 15(UV) 18
+              32:     31(ptr) AccessChain 30 26
+              33:          27 Load 32
+              35:   13(fvec2) Load 34(aPos)
+              38:    6(float) CompositeExtract 35 0
+              39:    6(float) CompositeExtract 35 1
+              40:    7(fvec4) CompositeConstruct 38 39 36 37
+              41:    7(fvec4) MatrixTimesVector 33 40
+              42:      8(ptr) AccessChain 24 26
+                              Store 42 41
+                              Return
+                              FunctionEnd
+// Module Version 10000
+// Generated by (magic number): 8000a
+// Id's are bound by 27
+
+                              Capability Shader
+               1:             ExtInstImport  "GLSL.std.450"
+                              MemoryModel Logical GLSL450
+                              EntryPoint Fragment 4  "main" 9 11 20
+                              ExecutionMode 4 OriginUpperLeft
+                              Source GLSL 460
+                              Name 4  "main"
+                              Name 9  "fragColor"
+                              Name 11  "Color"
+                              Name 16  "sTexture"
+                              Name 20  "UV"
+                              Decorate 9(fragColor) Location 0
+                              Decorate 11(Color) Location 0
+                              Decorate 16(sTexture) DescriptorSet 1
+                              Decorate 16(sTexture) Binding 0
+                              Decorate 20(UV) Location 1
+               2:             TypeVoid
+               3:             TypeFunction 2
+               6:             TypeFloat 32
+               7:             TypeVector 6(float) 4
+               8:             TypePointer Output 7(fvec4)
+    9(fragColor):      8(ptr) Variable Output
+              10:             TypePointer Input 7(fvec4)
+       11(Color):     10(ptr) Variable Input
+              13:             TypeImage 6(float) 2D sampled format:Unknown
+              14:             TypeSampledImage 13
+              15:             TypePointer UniformConstant 14
+    16(sTexture):     15(ptr) Variable UniformConstant
+              18:             TypeVector 6(float) 2
+              19:             TypePointer Input 18(fvec2)
+          20(UV):     19(ptr) Variable Input
+              23:             TypeInt 32 0
+              24:     23(int) Constant 0
+         4(main):           2 Function None 3
+               5:             Label
+              12:    7(fvec4) Load 11(Color)
+              17:          14 Load 16(sTexture)
+              21:   18(fvec2) Load 20(UV)
+              22:    7(fvec4) ImageSampleImplicitLod 17 21
+              25:    6(float) CompositeExtract 22 0
+              26:    7(fvec4) VectorTimesScalar 12 25
+                              Store 9(fragColor) 26
+                              Return
+                              FunctionEnd
diff --git a/Test/vk.relaxed.changeSet.frag b/Test/vk.relaxed.changeSet.frag
new file mode 100755 (executable)
index 0000000..e2f2403
--- /dev/null
@@ -0,0 +1,13 @@
+#version 460
+
+layout(location = 0) out vec4 fragColor;
+
+uniform sampler2D sTexture;
+
+in vec4 Color;
+in vec2 UV;
+
+void main()
+{
+    fragColor = Color * texture(sTexture, UV.st).r;
+}
diff --git a/Test/vk.relaxed.changeSet.vert b/Test/vk.relaxed.changeSet.vert
new file mode 100755 (executable)
index 0000000..419adba
--- /dev/null
@@ -0,0 +1,16 @@
+#version 460
+
+in vec2 aPos;
+in vec2 aUV;
+in vec4 aColor;
+uniform mat4 projectionMatrix;
+
+out vec4 Color;
+out vec2 UV;
+
+void main()
+{
+    Color = aColor;
+    UV = aUV;
+    gl_Position = projectionMatrix * vec4(aPos, 0, 1);
+}
index e85196c44511d3e6885635e292aabe04ff6a1ddc..d02eae6fcc75364323dcc40405783032584156ca 100644 (file)
@@ -2124,7 +2124,12 @@ bool TProgram::crossStageCheck(EShMessages) {
 
     // copy final definition of global block back into each stage
     for (unsigned int i = 0; i < activeStages.size(); ++i) {
-        activeStages[i]->mergeGlobalUniformBlocks(*infoSink, uniforms);
+        // We only want to merge into already existing global uniform blocks.
+        // A stage that doesn't already know about the global doesn't care about it's content.
+        // Otherwise we end up pointing to the same object between different stages
+        // and that will break binding/set remappings
+        bool mergeExistingOnly = true;
+        activeStages[i]->mergeGlobalUniformBlocks(*infoSink, uniforms, mergeExistingOnly);
     }
 
     // compare cross stage symbols for each stage boundary
index 789dc3c30849557370482ecf1cb37abe4bee7d97..33af7bbe0958d5931f4723f56690c08e2671e528 100644 (file)
@@ -108,7 +108,8 @@ void TIntermediate::mergeUniformObjects(TInfoSink& infoSink, TIntermediate& unit
     unitLinkerObjects.resize(end - unitLinkerObjects.begin());
 
     // merge uniforms and do error checking
-    mergeGlobalUniformBlocks(infoSink, unit);
+    bool mergeExistingOnly = false;
+    mergeGlobalUniformBlocks(infoSink, unit, mergeExistingOnly);
     mergeLinkerObjects(infoSink, linkerObjects, unitLinkerObjects, unit.getStage());
 }
 
@@ -362,7 +363,8 @@ void TIntermediate::mergeTrees(TInfoSink& infoSink, TIntermediate& unit)
     remapIds(idMaps, idShift + 1, unit);
 
     mergeBodies(infoSink, globals, unitGlobals);
-    mergeGlobalUniformBlocks(infoSink, unit);
+    bool mergeExistingOnly = false;
+    mergeGlobalUniformBlocks(infoSink, unit, mergeExistingOnly);
     mergeLinkerObjects(infoSink, linkerObjects, unitLinkerObjects, unit.getStage());
     ioAccessed.insert(unit.ioAccessed.begin(), unit.ioAccessed.end());
 }
@@ -525,7 +527,7 @@ static inline bool isSameInterface(TIntermSymbol* symbol, EShLanguage stage, TIn
 // merge the members of different stages to allow them to be linked properly
 // as a single block
 //
-void TIntermediate::mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit)
+void TIntermediate::mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit, bool mergeExistingOnly)
 {
     TIntermSequence& linkerObjects = findLinkerObjects()->getSequence();
     TIntermSequence& unitLinkerObjects = unit.findLinkerObjects()->getSequence();
@@ -552,7 +554,7 @@ void TIntermediate::mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate&
     auto itUnitBlock = unitDefaultBlocks.begin();
     for (; itUnitBlock != unitDefaultBlocks.end(); itUnitBlock++) {
 
-        bool add = true;
+        bool add = !mergeExistingOnly;
         auto itBlock = defaultBlocks.begin();
 
         for (; itBlock != defaultBlocks.end(); itBlock++) {
index 9bfa35cc885290a4f96b1712fd62882756108aec..c9a1d8114a4d15407812b020078a50456a180bfb 100644 (file)
@@ -915,7 +915,7 @@ public:
     void merge(TInfoSink&, TIntermediate&);
     void finalCheck(TInfoSink&, bool keepUncalled);
 
-    void mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit);
+    void mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit, bool mergeExistingOnly);
     void mergeUniformObjects(TInfoSink& infoSink, TIntermediate& unit);
     void checkStageIO(TInfoSink&, TIntermediate&);
 
index d791d6cc6750a1c44a5b34d37b03ca632f211bb7..32e3c29b886af84d1eaf7cb9ba2a1685b3892e6c 100644 (file)
@@ -48,6 +48,7 @@ namespace {
 
 struct vkRelaxedData {
     std::vector<std::string> fileNames;
+    std::vector<std::vector<std::string>> resourceSetBindings;
 };
 
 using VulkanRelaxedTest = GlslangTest <::testing::TestWithParam<vkRelaxedData>>;
@@ -191,6 +192,7 @@ bool verifyIOMapping(std::string& linkingError, glslang::TProgram& program) {
 TEST_P(VulkanRelaxedTest, FromFile)
 {
     const auto& fileNames = GetParam().fileNames;
+    const auto& resourceSetBindings = GetParam().resourceSetBindings;
     Semantics semantics = Semantics::Vulkan;
     const size_t fileCount = fileNames.size();
     const EShMessages controls = DeriveOptions(Source::GLSL, semantics, Target::BothASTAndSpv);
@@ -230,6 +232,12 @@ TEST_P(VulkanRelaxedTest, FromFile)
     result.linkingOutput = program.getInfoLog();
     result.linkingError = program.getInfoDebugLog();
 
+    if (!resourceSetBindings.empty()) {
+        assert(resourceSetBindings.size() == fileNames.size());
+        for (int i = 0; i < shaders.size(); i++)
+            shaders[i]->setResourceSetBinding(resourceSetBindings[i]);
+    }
+
     unsigned int stage = 0;
     glslang::TIntermediate* firstIntermediate = nullptr;
     while (!program.getIntermediate((EShLanguage)stage) && stage < EShLangCount) { stage++; }
@@ -287,6 +295,7 @@ INSTANTIATE_TEST_SUITE_P(
         {{"vk.relaxed.link1.frag", "vk.relaxed.link2.frag"}},
         {{"vk.relaxed.stagelink.vert", "vk.relaxed.stagelink.frag"}},
         {{"vk.relaxed.errorcheck.vert", "vk.relaxed.errorcheck.frag"}},
+        {{"vk.relaxed.changeSet.vert", "vk.relaxed.changeSet.frag" }, { {"0"}, {"1"} } },
     }))
 );
 // clang-format on