From 76117921b9b1ff6e49eab26ed38f6fbda687c7a7 Mon Sep 17 00:00:00 2001 From: LoopDawg Date: Wed, 6 Sep 2017 14:59:06 -0600 Subject: [PATCH] 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. --- SPIRV/GlslangToSpv.cpp | 11 ++- Test/baseResults/hlsl.opaque-type-bug.frag.out | 111 +++++++++++++++++++++++++ Test/hlsl.opaque-type-bug.frag | 16 ++++ gtests/Hlsl.FromFile.cpp | 1 + 4 files changed, 136 insertions(+), 3 deletions(-) create mode 100644 Test/baseResults/hlsl.opaque-type-bug.frag.out create mode 100644 Test/hlsl.opaque-type-bug.frag diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index b6c9705..a82867a 100755 --- 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(¶mType); // 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 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 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 --- 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"}, -- 2.7.4