Fix lvalue check in SPIR-V generation

There were several locations in TGlslangToSpvTraverser::handleUserFunctionCall testing for
whether a fn argument should be in the lvalue or rvalue array.  They must get the same
result for indexing sanity, but had slightly different logic.

They're now forced into the same test.

diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp
index b6c9705..a82867a 100755 (executable)
--- a/SPIRV/GlslangToSpv.cpp
+++ b/SPIRV/GlslangToSpv.cpp
@@ -3529,6 +3529,11 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg
     const glslang::TIntermSequence& glslangArgs = node->getSequence();
     const glslang::TQualifierList& qualifiers = node->getQualifierList();
 
+    // Encapsulate lvalue logic, used in two places below, for safety.
+    const auto isLValue = [](int qualifier, const glslang::TType& paramType) -> bool {
+        return qualifier != glslang::EvqConstReadOnly || paramType.containsOpaque();
+    };
+
     //  See comments in makeFunctions() for details about the semantics for parameter passing.
     //
     // These imply we need a four step process:
@@ -3548,7 +3553,7 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg
         glslangArgs[a]->traverse(this);
         argTypes.push_back(&paramType);
         // keep outputs and opaque objects as l-values, evaluate input-only as r-values
-        if (qualifiers[a] != glslang::EvqConstReadOnly || paramType.containsOpaque()) {
+        if (isLValue(qualifiers[a], paramType)) {
             // save l-value
             lValues.push_back(builder.getAccessChain());
         } else {
@@ -3573,7 +3578,7 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg
             builder.setAccessChain(lValues[lValueCount]);
             arg = builder.accessChainGetLValue();
             ++lValueCount;
-        } else if (qualifiers[a] != glslang::EvqConstReadOnly) {
+        } else if (isLValue(qualifiers[a], paramType)) {
             // need space to hold the copy
             arg = builder.createVariable(spv::StorageClassFunction, convertGlslangToSpvType(paramType), "param");
             if (qualifiers[a] == glslang::EvqIn || qualifiers[a] == glslang::EvqInOut) {
@@ -3600,7 +3605,7 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg
     lValueCount = 0;
     for (int a = 0; a < (int)glslangArgs.size(); ++a) {
         const glslang::TType& paramType = glslangArgs[a]->getAsTyped()->getType();
-        if (qualifiers[a] != glslang::EvqConstReadOnly) {
+        if (isLValue(qualifiers[a], paramType)) {
             if (qualifiers[a] == glslang::EvqOut || qualifiers[a] == glslang::EvqInOut) {
                 spv::Id copy = builder.createLoad(spvArgs[a]);
                 builder.setAccessChain(lValues[lValueCount]);

diff --git a/Test/baseResults/hlsl.opaque-type-bug.frag.out b/Test/baseResults/hlsl.opaque-type-bug.frag.out
new file mode 100644 (file)
index 0000000..3eed752
--- /dev/null
+++ b/Test/baseResults/hlsl.opaque-type-bug.frag.out
@@ -0,0 +1,111 @@
+hlsl.opaque-type-bug.frag
+Shader version: 500
+gl_FragCoord origin is upper left
+0:? Sequence
+0:6  Function Definition: TexFunc(t21;vf3; ( temp void)
+0:6    Function Parameters: 
+0:6      't2D' ( const (read only) texture2D)
+0:6      'RGB' ( out 3-component vector of float)
+0:?     Sequence
+0:7      move second child to first child ( temp 3-component vector of float)
+0:7        'RGB' ( out 3-component vector of float)
+0:7        Constant:
+0:7          0.000000
+0:7          0.000000
+0:7          0.000000
+0:12  Function Definition: @main( ( temp void)
+0:12    Function Parameters: 
+0:?     Sequence
+0:15      Function Call: TexFunc(t21;vf3; ( temp void)
+0:15        'MyTexture' (layout( binding=0) uniform texture2D)
+0:15        'final_RGB' ( temp 3-component vector of float)
+0:12  Function Definition: main( ( temp void)
+0:12    Function Parameters: 
+0:?     Sequence
+0:12      Function Call: @main( ( temp void)
+0:?   Linker Objects
+0:?     'MyTexture' (layout( binding=0) uniform texture2D)
+
+
+Linked fragment stage:
+
+
+Shader version: 500
+gl_FragCoord origin is upper left
+0:? Sequence
+0:6  Function Definition: TexFunc(t21;vf3; ( temp void)
+0:6    Function Parameters: 
+0:6      't2D' ( const (read only) texture2D)
+0:6      'RGB' ( out 3-component vector of float)
+0:?     Sequence
+0:7      move second child to first child ( temp 3-component vector of float)
+0:7        'RGB' ( out 3-component vector of float)
+0:7        Constant:
+0:7          0.000000
+0:7          0.000000
+0:7          0.000000
+0:12  Function Definition: @main( ( temp void)
+0:12    Function Parameters: 
+0:?     Sequence
+0:15      Function Call: TexFunc(t21;vf3; ( temp void)
+0:15        'MyTexture' (layout( binding=0) uniform texture2D)
+0:15        'final_RGB' ( temp 3-component vector of float)
+0:12  Function Definition: main( ( temp void)
+0:12    Function Parameters: 
+0:?     Sequence
+0:12      Function Call: @main( ( temp void)
+0:?   Linker Objects
+0:?     'MyTexture' (layout( binding=0) uniform texture2D)
+
+// Module Version 10000
+// Generated by (magic number): 80001
+// Id's are bound by 26
+
+                              Capability Shader
+               1:             ExtInstImport  "GLSL.std.450"
+                              MemoryModel Logical GLSL450
+                              EntryPoint Fragment 4  "main"
+                              ExecutionMode 4 OriginUpperLeft
+                              Source HLSL 500
+                              Name 4  "main"
+                              Name 14  "TexFunc(t21;vf3;"
+                              Name 12  "t2D"
+                              Name 13  "RGB"
+                              Name 16  "@main("
+                              Name 20  "MyTexture"
+                              Name 21  "final_RGB"
+                              Name 22  "param"
+                              Decorate 20(MyTexture) DescriptorSet 0
+                              Decorate 20(MyTexture) Binding 0
+               2:             TypeVoid
+               3:             TypeFunction 2
+               6:             TypeFloat 32
+               7:             TypeImage 6(float) 2D sampled format:Unknown
+               8:             TypePointer UniformConstant 7
+               9:             TypeVector 6(float) 3
+              10:             TypePointer Function 9(fvec3)
+              11:             TypeFunction 2 8(ptr) 10(ptr)
+              18:    6(float) Constant 0
+              19:    9(fvec3) ConstantComposite 18 18 18
+   20(MyTexture):      8(ptr) Variable UniformConstant
+         4(main):           2 Function None 3
+               5:             Label
+              25:           2 FunctionCall 16(@main()
+                              Return
+                              FunctionEnd
+14(TexFunc(t21;vf3;):           2 Function None 11
+         12(t2D):      8(ptr) FunctionParameter
+         13(RGB):     10(ptr) FunctionParameter
+              15:             Label
+                              Store 13(RGB) 19
+                              Return
+                              FunctionEnd
+      16(@main():           2 Function None 3
+              17:             Label
+   21(final_RGB):     10(ptr) Variable Function
+       22(param):     10(ptr) Variable Function
+              23:           2 FunctionCall 14(TexFunc(t21;vf3;) 20(MyTexture) 22(param)
+              24:    9(fvec3) Load 22(param)
+                              Store 21(final_RGB) 24
+                              Return
+                              FunctionEnd

diff --git a/Test/hlsl.opaque-type-bug.frag b/Test/hlsl.opaque-type-bug.frag
new file mode 100644 (file)
index 0000000..f4ccaea
--- /dev/null
+++ b/Test/hlsl.opaque-type-bug.frag
@@ -0,0 +1,16 @@
+
+Texture2D      MyTexture : register(t0);
+
+//----------------------------------------------------------------------------------------
+void TexFunc(in const Texture2D t2D, out float3 RGB)
+{    
+    RGB = 0;
+}
+
+//-----------------------------------------------------------------------------------
+void main() 
+{
+    float3 final_RGB;
+
+    TexFunc(MyTexture, final_RGB);
+}

diff --git a/gtests/Hlsl.FromFile.cpp b/gtests/Hlsl.FromFile.cpp
index 4079d83..4dfe7d7 100644 (file)
--- a/gtests/Hlsl.FromFile.cpp
+++ b/gtests/Hlsl.FromFile.cpp
@@ -226,6 +226,7 @@ INSTANTIATE_TEST_CASE_P(
         {"hlsl.numericsuffixes.frag", "main"},
         {"hlsl.numthreads.comp", "main_aux1"},
         {"hlsl.overload.frag", "PixelShaderFunction"},
+        {"hlsl.opaque-type-bug.frag", "main"},
         {"hlsl.params.default.frag", "main"},
         {"hlsl.params.default.negative.frag", "main"},
         {"hlsl.partialInit.frag", "PixelShaderFunction"},