From 11c24e9adbd915de7651a447a85d69a8a51bfdc0 Mon Sep 17 00:00:00 2001 From: Greg Fischer Date: Thu, 20 May 2021 09:17:53 -0600 Subject: [PATCH] Do true SPV type check for function array arg linkage Previous check was missing type difference between uniform array actual arg with stride decoration and the formal arg without. Now does logical or component-wise copy where needed. Fixes #2637 --- SPIRV/GlslangToSpv.cpp | 2 +- .../spv.1.4.OpCopyLogical.funcall.frag.out | 90 +++++++-------- .../spv.1.4.funcall.array.frag.out | 74 ++++++++++++ Test/baseResults/spv.funcall.array.frag.out | 106 ++++++++++++++++++ .../spv.multiStructFuncall.frag.out | 100 ++++++++--------- Test/spv.1.4.funcall.array.frag | 17 +++ Test/spv.funcall.array.frag | 17 +++ gtests/Spv.FromFile.cpp | 2 + 8 files changed, 308 insertions(+), 100 deletions(-) create mode 100644 Test/baseResults/spv.1.4.funcall.array.frag.out create mode 100644 Test/baseResults/spv.funcall.array.frag.out create mode 100644 Test/spv.1.4.funcall.array.frag create mode 100644 Test/spv.funcall.array.frag diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index 1d8f19945..bf9ac4316 100644 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -5562,7 +5562,7 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg ++lValueCount; } else { // process r-value, which involves a copy for a type mismatch - if (function->getParamType(a) != convertGlslangToSpvType(*argTypes[a]) || + if (function->getParamType(a) != builder.getTypeId(rValues[rValueCount]) || TranslatePrecisionDecoration(*argTypes[a]) != function->getParamPrecision(a)) { spv::Id argCopy = builder.createVariable(function->getParamPrecision(a), spv::StorageClassFunction, function->getParamType(a), "arg"); diff --git a/Test/baseResults/spv.1.4.OpCopyLogical.funcall.frag.out b/Test/baseResults/spv.1.4.OpCopyLogical.funcall.frag.out index 68f7e520e..a2458bafe 100644 --- a/Test/baseResults/spv.1.4.OpCopyLogical.funcall.frag.out +++ b/Test/baseResults/spv.1.4.OpCopyLogical.funcall.frag.out @@ -1,12 +1,12 @@ spv.1.4.OpCopyLogical.funcall.frag // Module Version 10400 // Generated by (magic number): 8000a -// Id's are bound by 60 +// Id's are bound by 59 Capability Shader 1: ExtInstImport "GLSL.std.450" MemoryModel Logical GLSL450 - EntryPoint Fragment 4 "main" 25 37 + EntryPoint Fragment 4 "main" 25 36 ExecutionMode 4 OriginUpperLeft Source GLSL 450 Name 4 "main" @@ -23,14 +23,12 @@ spv.1.4.OpCopyLogical.funcall.frag Name 23 "blockName" MemberName 23(blockName) 0 "s1" Name 25 "" - Name 31 "S" - MemberName 31(S) 0 "m" - Name 32 "arg" - Name 37 "s2" - Name 40 "param" - Name 45 "param" - Name 48 "param" - Name 56 "param" + Name 31 "arg" + Name 36 "s2" + Name 39 "param" + Name 44 "param" + Name 47 "param" + Name 55 "param" MemberDecorate 22(S) 0 ColMajor MemberDecorate 22(S) 0 Offset 0 MemberDecorate 22(S) 0 MatrixStride 16 @@ -38,7 +36,6 @@ spv.1.4.OpCopyLogical.funcall.frag Decorate 23(blockName) Block Decorate 25 DescriptorSet 0 Decorate 25 Binding 0 - MemberDecorate 31(S) 0 ColMajor 2: TypeVoid 3: TypeFunction 2 6: TypeFloat 32 @@ -55,46 +52,45 @@ spv.1.4.OpCopyLogical.funcall.frag 26: TypeInt 32 1 27: 26(int) Constant 0 28: TypePointer StorageBuffer 22(S) - 31(S): TypeStruct 8 - 36: TypePointer Private 9(S) - 37(s2): 36(ptr) Variable Private + 35: TypePointer Private 9(S) + 36(s2): 35(ptr) Variable Private 4(main): 2 Function None 3 5: Label - 32(arg): 14(ptr) Variable Function - 40(param): 14(ptr) Variable Function - 45(param): 14(ptr) Variable Function - 48(param): 14(ptr) Variable Function - 56(param): 14(ptr) Variable Function + 31(arg): 14(ptr) Variable Function + 39(param): 14(ptr) Variable Function + 44(param): 14(ptr) Variable Function + 47(param): 14(ptr) Variable Function + 55(param): 14(ptr) Variable Function 29: 28(ptr) AccessChain 25 27 30: 22(S) Load 29 - 33: 9(S) CopyLogical 30 - Store 32(arg) 33 - 34: 9(S) Load 32(arg) - 35: 2 FunctionCall 12(fooConst(struct-S-mf441;) 34 - 38: 9(S) Load 37(s2) - 39: 2 FunctionCall 12(fooConst(struct-S-mf441;) 38 - 41: 28(ptr) AccessChain 25 27 - 42: 22(S) Load 41 - 43: 9(S) CopyLogical 42 - Store 40(param) 43 - 44: 2 FunctionCall 17(foo(struct-S-mf441;) 40(param) - 46: 9(S) Load 37(s2) - Store 45(param) 46 - 47: 2 FunctionCall 17(foo(struct-S-mf441;) 45(param) - 49: 28(ptr) AccessChain 25 27 - 50: 22(S) Load 49 - 51: 9(S) CopyLogical 50 - Store 48(param) 51 - 52: 2 FunctionCall 20(fooOut(struct-S-mf441;) 48(param) - 53: 9(S) Load 48(param) - 54: 28(ptr) AccessChain 25 27 - 55: 22(S) CopyLogical 53 - Store 54 55 - 57: 9(S) Load 37(s2) - Store 56(param) 57 - 58: 2 FunctionCall 20(fooOut(struct-S-mf441;) 56(param) - 59: 9(S) Load 56(param) - Store 37(s2) 59 + 32: 9(S) CopyLogical 30 + Store 31(arg) 32 + 33: 9(S) Load 31(arg) + 34: 2 FunctionCall 12(fooConst(struct-S-mf441;) 33 + 37: 9(S) Load 36(s2) + 38: 2 FunctionCall 12(fooConst(struct-S-mf441;) 37 + 40: 28(ptr) AccessChain 25 27 + 41: 22(S) Load 40 + 42: 9(S) CopyLogical 41 + Store 39(param) 42 + 43: 2 FunctionCall 17(foo(struct-S-mf441;) 39(param) + 45: 9(S) Load 36(s2) + Store 44(param) 45 + 46: 2 FunctionCall 17(foo(struct-S-mf441;) 44(param) + 48: 28(ptr) AccessChain 25 27 + 49: 22(S) Load 48 + 50: 9(S) CopyLogical 49 + Store 47(param) 50 + 51: 2 FunctionCall 20(fooOut(struct-S-mf441;) 47(param) + 52: 9(S) Load 47(param) + 53: 28(ptr) AccessChain 25 27 + 54: 22(S) CopyLogical 52 + Store 53 54 + 56: 9(S) Load 36(s2) + Store 55(param) 56 + 57: 2 FunctionCall 20(fooOut(struct-S-mf441;) 55(param) + 58: 9(S) Load 55(param) + Store 36(s2) 58 Return FunctionEnd 12(fooConst(struct-S-mf441;): 2 Function None 10 diff --git a/Test/baseResults/spv.1.4.funcall.array.frag.out b/Test/baseResults/spv.1.4.funcall.array.frag.out new file mode 100644 index 000000000..d976bb1f4 --- /dev/null +++ b/Test/baseResults/spv.1.4.funcall.array.frag.out @@ -0,0 +1,74 @@ +spv.1.4.funcall.array.frag +// Module Version 10400 +// Generated by (magic number): 8000a +// Id's are bound by 42 + + Capability Shader + 1: ExtInstImport "GLSL.std.450" + MemoryModel Logical GLSL450 + EntryPoint Fragment 4 "main" 27 31 + ExecutionMode 4 OriginUpperLeft + Source GLSL 450 + Name 4 "main" + Name 16 "f(vf4[9];i1;" + Name 14 "a" + Name 15 "ix" + Name 20 "indexable" + Name 27 "color" + Name 29 "ub" + MemberName 29(ub) 0 "u" + Name 31 "" + Name 37 "arg" + Name 40 "param" + Decorate 27(color) Location 0 + Decorate 28 ArrayStride 16 + MemberDecorate 29(ub) 0 Offset 0 + Decorate 29(ub) Block + Decorate 31 DescriptorSet 0 + Decorate 31 Binding 0 + 2: TypeVoid + 3: TypeFunction 2 + 6: TypeFloat 32 + 7: TypeVector 6(float) 4 + 8: TypeInt 32 0 + 9: 8(int) Constant 9 + 10: TypeArray 7(fvec4) 9 + 11: TypeInt 32 1 + 12: TypePointer Function 11(int) + 13: TypeFunction 7(fvec4) 10 12(ptr) + 19: TypePointer Function 10 + 21: TypePointer Function 7(fvec4) + 26: TypePointer Output 7(fvec4) + 27(color): 26(ptr) Variable Output + 28: TypeArray 7(fvec4) 9 + 29(ub): TypeStruct 28 + 30: TypePointer Uniform 29(ub) + 31: 30(ptr) Variable Uniform + 32: 11(int) Constant 0 + 33: TypePointer Uniform 28 + 36: 11(int) Constant 2 + 4(main): 2 Function None 3 + 5: Label + 37(arg): 19(ptr) Variable Function + 40(param): 12(ptr) Variable Function + 34: 33(ptr) AccessChain 31 32 + 35: 28 Load 34 + 38: 10 CopyLogical 35 + Store 37(arg) 38 + 39: 10 Load 37(arg) + Store 40(param) 36 + 41: 7(fvec4) FunctionCall 16(f(vf4[9];i1;) 39 40(param) + Store 27(color) 41 + Return + FunctionEnd +16(f(vf4[9];i1;): 7(fvec4) Function None 13 + 14(a): 10 FunctionParameter + 15(ix): 12(ptr) FunctionParameter + 17: Label + 20(indexable): 19(ptr) Variable Function + 18: 11(int) Load 15(ix) + Store 20(indexable) 14(a) + 22: 21(ptr) AccessChain 20(indexable) 18 + 23: 7(fvec4) Load 22 + ReturnValue 23 + FunctionEnd diff --git a/Test/baseResults/spv.funcall.array.frag.out b/Test/baseResults/spv.funcall.array.frag.out new file mode 100644 index 000000000..616ba16c0 --- /dev/null +++ b/Test/baseResults/spv.funcall.array.frag.out @@ -0,0 +1,106 @@ +spv.funcall.array.frag +// Module Version 10000 +// Generated by (magic number): 8000a +// Id's are bound by 66 + + Capability Shader + 1: ExtInstImport "GLSL.std.450" + MemoryModel Logical GLSL450 + EntryPoint Fragment 4 "main" 27 + ExecutionMode 4 OriginUpperLeft + Source GLSL 450 + Name 4 "main" + Name 16 "f(vf4[9];i1;" + Name 14 "a" + Name 15 "ix" + Name 20 "indexable" + Name 27 "color" + Name 29 "ub" + MemberName 29(ub) 0 "u" + Name 31 "" + Name 37 "arg" + Name 64 "param" + Decorate 27(color) Location 0 + Decorate 28 ArrayStride 16 + MemberDecorate 29(ub) 0 Offset 0 + Decorate 29(ub) Block + Decorate 31 DescriptorSet 0 + Decorate 31 Binding 0 + 2: TypeVoid + 3: TypeFunction 2 + 6: TypeFloat 32 + 7: TypeVector 6(float) 4 + 8: TypeInt 32 0 + 9: 8(int) Constant 9 + 10: TypeArray 7(fvec4) 9 + 11: TypeInt 32 1 + 12: TypePointer Function 11(int) + 13: TypeFunction 7(fvec4) 10 12(ptr) + 19: TypePointer Function 10 + 21: TypePointer Function 7(fvec4) + 26: TypePointer Output 7(fvec4) + 27(color): 26(ptr) Variable Output + 28: TypeArray 7(fvec4) 9 + 29(ub): TypeStruct 28 + 30: TypePointer Uniform 29(ub) + 31: 30(ptr) Variable Uniform + 32: 11(int) Constant 0 + 33: TypePointer Uniform 28 + 36: 11(int) Constant 2 + 41: 11(int) Constant 1 + 46: 11(int) Constant 3 + 49: 11(int) Constant 4 + 52: 11(int) Constant 5 + 55: 11(int) Constant 6 + 58: 11(int) Constant 7 + 61: 11(int) Constant 8 + 4(main): 2 Function None 3 + 5: Label + 37(arg): 19(ptr) Variable Function + 64(param): 12(ptr) Variable Function + 34: 33(ptr) AccessChain 31 32 + 35: 28 Load 34 + 38: 7(fvec4) CompositeExtract 35 0 + 39: 21(ptr) AccessChain 37(arg) 32 + Store 39 38 + 40: 7(fvec4) CompositeExtract 35 1 + 42: 21(ptr) AccessChain 37(arg) 41 + Store 42 40 + 43: 7(fvec4) CompositeExtract 35 2 + 44: 21(ptr) AccessChain 37(arg) 36 + Store 44 43 + 45: 7(fvec4) CompositeExtract 35 3 + 47: 21(ptr) AccessChain 37(arg) 46 + Store 47 45 + 48: 7(fvec4) CompositeExtract 35 4 + 50: 21(ptr) AccessChain 37(arg) 49 + Store 50 48 + 51: 7(fvec4) CompositeExtract 35 5 + 53: 21(ptr) AccessChain 37(arg) 52 + Store 53 51 + 54: 7(fvec4) CompositeExtract 35 6 + 56: 21(ptr) AccessChain 37(arg) 55 + Store 56 54 + 57: 7(fvec4) CompositeExtract 35 7 + 59: 21(ptr) AccessChain 37(arg) 58 + Store 59 57 + 60: 7(fvec4) CompositeExtract 35 8 + 62: 21(ptr) AccessChain 37(arg) 61 + Store 62 60 + 63: 10 Load 37(arg) + Store 64(param) 36 + 65: 7(fvec4) FunctionCall 16(f(vf4[9];i1;) 63 64(param) + Store 27(color) 65 + Return + FunctionEnd +16(f(vf4[9];i1;): 7(fvec4) Function None 13 + 14(a): 10 FunctionParameter + 15(ix): 12(ptr) FunctionParameter + 17: Label + 20(indexable): 19(ptr) Variable Function + 18: 11(int) Load 15(ix) + Store 20(indexable) 14(a) + 22: 21(ptr) AccessChain 20(indexable) 18 + 23: 7(fvec4) Load 22 + ReturnValue 23 + FunctionEnd diff --git a/Test/baseResults/spv.multiStructFuncall.frag.out b/Test/baseResults/spv.multiStructFuncall.frag.out index dd57cd1c1..eec734a35 100644 --- a/Test/baseResults/spv.multiStructFuncall.frag.out +++ b/Test/baseResults/spv.multiStructFuncall.frag.out @@ -1,7 +1,7 @@ spv.multiStructFuncall.frag // Module Version 10000 // Generated by (magic number): 8000a -// Id's are bound by 66 +// Id's are bound by 65 Capability Shader 1: ExtInstImport "GLSL.std.450" @@ -23,14 +23,12 @@ spv.multiStructFuncall.frag Name 23 "blockName" MemberName 23(blockName) 0 "s1" Name 25 "" - Name 31 "S" - MemberName 31(S) 0 "m" - Name 32 "arg" - Name 39 "s2" - Name 42 "param" - Name 48 "param" - Name 51 "param" - Name 62 "param" + Name 31 "arg" + Name 38 "s2" + Name 41 "param" + Name 47 "param" + Name 50 "param" + Name 61 "param" MemberDecorate 22(S) 0 ColMajor MemberDecorate 22(S) 0 Offset 0 MemberDecorate 22(S) 0 MatrixStride 16 @@ -38,7 +36,6 @@ spv.multiStructFuncall.frag Decorate 23(blockName) BufferBlock Decorate 25 DescriptorSet 0 Decorate 25 Binding 0 - MemberDecorate 31(S) 0 ColMajor 2: TypeVoid 3: TypeFunction 2 6: TypeFloat 32 @@ -55,52 +52,51 @@ spv.multiStructFuncall.frag 26: TypeInt 32 1 27: 26(int) Constant 0 28: TypePointer Uniform 22(S) - 31(S): TypeStruct 8 - 34: TypePointer Function 8 - 38: TypePointer Private 9(S) - 39(s2): 38(ptr) Variable Private - 60: TypePointer Uniform 8 + 33: TypePointer Function 8 + 37: TypePointer Private 9(S) + 38(s2): 37(ptr) Variable Private + 59: TypePointer Uniform 8 4(main): 2 Function None 3 5: Label - 32(arg): 14(ptr) Variable Function - 42(param): 14(ptr) Variable Function - 48(param): 14(ptr) Variable Function - 51(param): 14(ptr) Variable Function - 62(param): 14(ptr) Variable Function + 31(arg): 14(ptr) Variable Function + 41(param): 14(ptr) Variable Function + 47(param): 14(ptr) Variable Function + 50(param): 14(ptr) Variable Function + 61(param): 14(ptr) Variable Function 29: 28(ptr) AccessChain 25 27 30: 22(S) Load 29 - 33: 8 CompositeExtract 30 0 - 35: 34(ptr) AccessChain 32(arg) 27 - Store 35 33 - 36: 9(S) Load 32(arg) - 37: 2 FunctionCall 12(fooConst(struct-S-mf441;) 36 - 40: 9(S) Load 39(s2) - 41: 2 FunctionCall 12(fooConst(struct-S-mf441;) 40 - 43: 28(ptr) AccessChain 25 27 - 44: 22(S) Load 43 - 45: 8 CompositeExtract 44 0 - 46: 34(ptr) AccessChain 42(param) 27 - Store 46 45 - 47: 2 FunctionCall 17(foo(struct-S-mf441;) 42(param) - 49: 9(S) Load 39(s2) - Store 48(param) 49 - 50: 2 FunctionCall 17(foo(struct-S-mf441;) 48(param) - 52: 28(ptr) AccessChain 25 27 - 53: 22(S) Load 52 - 54: 8 CompositeExtract 53 0 - 55: 34(ptr) AccessChain 51(param) 27 - Store 55 54 - 56: 2 FunctionCall 20(fooOut(struct-S-mf441;) 51(param) - 57: 9(S) Load 51(param) - 58: 28(ptr) AccessChain 25 27 - 59: 8 CompositeExtract 57 0 - 61: 60(ptr) AccessChain 58 27 - Store 61 59 - 63: 9(S) Load 39(s2) - Store 62(param) 63 - 64: 2 FunctionCall 20(fooOut(struct-S-mf441;) 62(param) - 65: 9(S) Load 62(param) - Store 39(s2) 65 + 32: 8 CompositeExtract 30 0 + 34: 33(ptr) AccessChain 31(arg) 27 + Store 34 32 + 35: 9(S) Load 31(arg) + 36: 2 FunctionCall 12(fooConst(struct-S-mf441;) 35 + 39: 9(S) Load 38(s2) + 40: 2 FunctionCall 12(fooConst(struct-S-mf441;) 39 + 42: 28(ptr) AccessChain 25 27 + 43: 22(S) Load 42 + 44: 8 CompositeExtract 43 0 + 45: 33(ptr) AccessChain 41(param) 27 + Store 45 44 + 46: 2 FunctionCall 17(foo(struct-S-mf441;) 41(param) + 48: 9(S) Load 38(s2) + Store 47(param) 48 + 49: 2 FunctionCall 17(foo(struct-S-mf441;) 47(param) + 51: 28(ptr) AccessChain 25 27 + 52: 22(S) Load 51 + 53: 8 CompositeExtract 52 0 + 54: 33(ptr) AccessChain 50(param) 27 + Store 54 53 + 55: 2 FunctionCall 20(fooOut(struct-S-mf441;) 50(param) + 56: 9(S) Load 50(param) + 57: 28(ptr) AccessChain 25 27 + 58: 8 CompositeExtract 56 0 + 60: 59(ptr) AccessChain 57 27 + Store 60 58 + 62: 9(S) Load 38(s2) + Store 61(param) 62 + 63: 2 FunctionCall 20(fooOut(struct-S-mf441;) 61(param) + 64: 9(S) Load 61(param) + Store 38(s2) 64 Return FunctionEnd 12(fooConst(struct-S-mf441;): 2 Function None 10 diff --git a/Test/spv.1.4.funcall.array.frag b/Test/spv.1.4.funcall.array.frag new file mode 100644 index 000000000..4f9727ba1 --- /dev/null +++ b/Test/spv.1.4.funcall.array.frag @@ -0,0 +1,17 @@ +#version 450 core + +uniform ub { + vec4 u[9]; +}; + +vec4 f(const vec4 a[9], int ix) { + return a[ix]; +} + +out vec4 color; + +void main() +{ + color = f(u, 2); +} + diff --git a/Test/spv.funcall.array.frag b/Test/spv.funcall.array.frag new file mode 100644 index 000000000..4f9727ba1 --- /dev/null +++ b/Test/spv.funcall.array.frag @@ -0,0 +1,17 @@ +#version 450 core + +uniform ub { + vec4 u[9]; +}; + +vec4 f(const vec4 a[9], int ix) { + return a[ix]; +} + +out vec4 color; + +void main() +{ + color = f(u, 2); +} + diff --git a/gtests/Spv.FromFile.cpp b/gtests/Spv.FromFile.cpp index 5456fb893..b679c53b8 100644 --- a/gtests/Spv.FromFile.cpp +++ b/gtests/Spv.FromFile.cpp @@ -351,6 +351,7 @@ INSTANTIATE_TEST_SUITE_P( "spv.functionSemantics.frag", "spv.functionParameterTypes.frag", "spv.GeometryShaderPassthrough.geom", + "spv.funcall.array.frag", "spv.interpOps.frag", "spv.int64.frag", "spv.intcoopmat.comp", @@ -555,6 +556,7 @@ INSTANTIATE_TEST_SUITE_P( "spv.1.4.OpCopyLogical.comp", "spv.1.4.OpCopyLogicalBool.comp", "spv.1.4.OpCopyLogical.funcall.frag", + "spv.1.4.funcall.array.frag", "spv.1.4.image.frag", "spv.1.4.sparseTexture.frag", "spv.1.4.texture.frag",