Merge pull request #518 from steve-lunarg/flatten-fix

Fix defects in uniform array flattening

Test/baseResults/hlsl.array.flatten.frag.out

@@ -8,8 +8,8 @@ gl_FragCoord origin is upper left
 0:18      Branch: Return with expression
 0:18        texture (global 4-component vector of float)
 0:18          Construct combined texture-sampler (temp sampler1D)
-0:?             'g_tex[1]' (temp texture1D)
+0:?             'g_tex[1]' (uniform texture1D)
-0:?             'g_samp[1]' (temp sampler)
+0:?             'g_samp[1]' (uniform sampler)
 0:18          Constant:
 0:18            0.200000
 0:22  Function Definition: TestFn2(t11[3];p1[3]; (global 4-component vector of float)
@@ -197,8 +197,8 @@ gl_FragCoord origin is upper left
 0:18      Branch: Return with expression
 0:18        texture (global 4-component vector of float)
 0:18          Construct combined texture-sampler (temp sampler1D)
-0:?             'g_tex[1]' (temp texture1D)
+0:?             'g_tex[1]' (uniform texture1D)
-0:?             'g_samp[1]' (temp sampler)
+0:?             'g_samp[1]' (uniform sampler)
 0:18          Constant:
 0:18            0.200000
 0:22  Function Definition: TestFn2(t11[3];p1[3]; (global 4-component vector of float)
@@ -419,6 +419,8 @@ gl_FragCoord origin is upper left
                               Name 125  "g_mats_explicit[1]"
                               Name 126  "g_mats_explicit[2]"
                               Name 127  "g_mats_explicit[3]"
+                              Decorate 36(g_tex[1]) DescriptorSet 0
+                              Decorate 39(g_samp[1]) DescriptorSet 0
                               Decorate 57(g_samp[0]) DescriptorSet 0
                               Decorate 62(g_samp[2]) DescriptorSet 0
                               Decorate 66(g_tex[0]) DescriptorSet 0

Test/baseResults/spv.register.autoassign-2.frag.out

@@ -0,0 +1,57 @@
+spv.register.autoassign-2.frag
+
+Linked fragment stage:
+
+
+// Module Version 10000
+// Generated by (magic number): 80001
+// Id's are bound by 30
+
+                              Capability Shader
+               1:             ExtInstImport  "GLSL.std.450"
+                              MemoryModel Logical GLSL450
+                              EntryPoint Fragment 4  "main" 9
+                              ExecutionMode 4 OriginUpperLeft
+                              Name 4  "main"
+                              Name 9  "Color"
+                              Name 12  "g_tScene[0]"
+                              Name 16  "g_tSamp"
+                              Name 24  "g_tScene[1]"
+                              Decorate 9(Color) Location 0
+                              Decorate 12(g_tScene[0]) DescriptorSet 0
+                              Decorate 12(g_tScene[0]) Binding 10
+                              Decorate 16(g_tSamp) DescriptorSet 0
+                              Decorate 16(g_tSamp) Binding 5
+                              Decorate 24(g_tScene[1]) DescriptorSet 0
+                              Decorate 24(g_tScene[1]) Binding 11
+               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:             TypeImage 6(float) 2D sampled format:Unknown
+              11:             TypePointer UniformConstant 10
+ 12(g_tScene[0]):     11(ptr) Variable UniformConstant
+              14:             TypeSampler
+              15:             TypePointer UniformConstant 14
+     16(g_tSamp):     15(ptr) Variable UniformConstant
+              18:             TypeSampledImage 10
+              20:             TypeVector 6(float) 2
+              21:    6(float) Constant 1050253722
+              22:   20(fvec2) ConstantComposite 21 21
+ 24(g_tScene[1]):     11(ptr) Variable UniformConstant
+         4(main):           2 Function None 3
+               5:             Label
+              13:          10 Load 12(g_tScene[0])
+              17:          14 Load 16(g_tSamp)
+              19:          18 SampledImage 13 17
+              23:    7(fvec4) ImageSampleImplicitLod 19 22
+              25:          10 Load 24(g_tScene[1])
+              26:          14 Load 16(g_tSamp)
+              27:          18 SampledImage 25 26
+              28:    7(fvec4) ImageSampleImplicitLod 27 22
+              29:    7(fvec4) FAdd 23 28
+                              Store 9(Color) 29
+                              Return
+                              FunctionEnd

Test/spv.register.autoassign-2.frag

@@ -0,0 +1,15 @@
+
+SamplerState g_tSamp : register(s0);
+
+Texture2D g_tScene[2] : register(t0);
+
+struct PS_OUTPUT
+{
+    float4 Color : SV_Target0;
+};
+
+void main(out PS_OUTPUT psout)
+{
+    psout.Color = g_tScene[0].Sample(g_tSamp, 0.3) +
+                  g_tScene[1].Sample(g_tSamp, 0.3);
+}

glslang/MachineIndependent/LiveTraverser.h

@@ -56,7 +56,9 @@ namespace glslang {
 
 class TLiveTraverser : public TIntermTraverser {
 public:
-    TLiveTraverser(const TIntermediate& i, bool traverseAll = false) :
+    TLiveTraverser(const TIntermediate& i, bool traverseAll = false,
+                   bool preVisit = true, bool inVisit = false, bool postVisit = false) :
+        TIntermTraverser(preVisit, inVisit, postVisit),
         intermediate(i), traverseAll(traverseAll)
     { }
 

glslang/MachineIndependent/iomapper.cpp

@@ -74,7 +74,7 @@ class TBindingTraverser : public TLiveTraverser {
 public:
     TBindingTraverser(const TIntermediate& i, TBindingMap& bindingMap, TUsedBindings& usedBindings,
                       bool traverseDeadCode = false) :
-        TLiveTraverser(i, traverseDeadCode),
+        TLiveTraverser(i, traverseDeadCode, true, true, false),
         bindingMap(bindingMap),
         usedBindings(usedBindings)
     { }

gtests/Spv.FromFile.cpp

@@ -48,6 +48,7 @@ struct IoMapData {
     int baseTextureBinding;
     int baseUboBinding;
     bool autoMapBindings;
+    bool flattenUniforms;
 };
 
 std::string FileNameAsCustomTestSuffixIoMap(
@@ -119,7 +120,8 @@ TEST_P(HlslSemantics, FromFile)
                                  GetParam().baseSamplerBinding,
                                  GetParam().baseTextureBinding,
                                  GetParam().baseUboBinding,
-                                 GetParam().autoMapBindings);
+                                 GetParam().autoMapBindings,
+                                 GetParam().flattenUniforms);
 }
 
 // clang-format off
@@ -251,8 +253,9 @@ INSTANTIATE_TEST_CASE_P(
 INSTANTIATE_TEST_CASE_P(
     Hlsl, HlslSemantics,
     ::testing::ValuesIn(std::vector<IoMapData>{
-        { "spv.register.autoassign.frag", "main_ep", 5, 10, 15, true },
+        { "spv.register.autoassign.frag", "main_ep", 5, 10, 15, true, false },
-        { "spv.register.noautoassign.frag", "main_ep", 5, 10, 15, false },
+        { "spv.register.noautoassign.frag", "main_ep", 5, 10, 15, false, false },
+        { "spv.register.autoassign-2.frag", "main", 5, 10, 15, true, true },
     }),
     FileNameAsCustomTestSuffixIoMap
 );

gtests/TestFixture.h

@@ -248,7 +248,8 @@ public:
             int baseSamplerBinding,
             int baseTextureBinding,
             int baseUboBinding,
-            bool autoMapBindings)
+            bool autoMapBindings,
+            bool flattenUniformArrays)
     {
         const EShLanguage kind = GetShaderStage(GetSuffix(shaderName));
 
@@ -257,6 +258,7 @@ public:
         shader.setShiftTextureBinding(baseTextureBinding);
         shader.setShiftUboBinding(baseUboBinding);
         shader.setAutoMapBindings(autoMapBindings);
+        shader.setFlattenUniformArrays(flattenUniformArrays);
 
         bool success = compile(&shader, code, entryPointName, controls);
 
@@ -433,7 +435,8 @@ public:
                                       int baseSamplerBinding,
                                       int baseTextureBinding,
                                       int baseUboBinding,
-                                      bool autoMapBindings)
+                                      bool autoMapBindings,
+                                      bool flattenUniformArrays)
     {
         const std::string inputFname = testDir + "/" + testName;
         const std::string expectedOutputFname =
@@ -446,7 +449,8 @@ public:
         const EShMessages controls = DeriveOptions(source, semantics, target);
         GlslangResult result = compileLinkIoMap(testName, input, entryPointName, controls,
                                                 baseSamplerBinding, baseTextureBinding, baseUboBinding,
-                                                autoMapBindings);
+                                                autoMapBindings,
+                                                flattenUniformArrays);
 
         // Generate the hybrid output in the way of glslangValidator.
         std::ostringstream stream;

hlsl/hlslParseHelper.cpp

@@ -383,6 +383,7 @@ TIntermTyped* HlslParseContext::handleBracketDereference(const TSourceLoc& loc,
 {
     TIntermTyped* result = nullptr;
 
+    bool flattened = false;
     int indexValue = 0;
     if (index->getQualifier().storage == EvqConst) {
         indexValue = index->getAsConstantUnion()->getConstArray()[0].getIConst();
@@ -408,6 +409,7 @@ TIntermTyped* HlslParseContext::handleBracketDereference(const TSourceLoc& loc,
                 error(loc, "Invalid variable index to flattened uniform array", base->getAsSymbolNode()->getName().c_str(), "");
 
             result = flattenAccess(base, indexValue);
+            flattened = (result != base);
         } else {
             if (index->getQualifier().storage == EvqConst) {
                 if (base->getType().isImplicitlySizedArray())
@@ -423,13 +425,18 @@ TIntermTyped* HlslParseContext::handleBracketDereference(const TSourceLoc& loc,
         // Insert dummy error-recovery result
         result = intermediate.addConstantUnion(0.0, EbtFloat, loc);
     } else {
-        // Insert valid dereferenced result
+        // If the array reference was flattened, it has the correct type.  E.g, if it was
-        TType newType(base->getType(), 0);  // dereferenced type
+        // a uniform array, it was flattened INTO a set of scalar uniforms, not scalar temps.
-        if (base->getType().getQualifier().storage == EvqConst && index->getQualifier().storage == EvqConst)
+        // In that case, we preserve the qualifiers.
-            newType.getQualifier().storage = EvqConst;
+        if (!flattened) {
-        else
+            // Insert valid dereferenced result
-            newType.getQualifier().storage = EvqTemporary;
+            TType newType(base->getType(), 0);  // dereferenced type
-        result->setType(newType);
+            if (base->getType().getQualifier().storage == EvqConst && index->getQualifier().storage == EvqConst)
+                newType.getQualifier().storage = EvqConst;
+            else
+                newType.getQualifier().storage = EvqTemporary;
+            result->setType(newType);
+        }
     }
 
     return result;