From 4df10335e6b5667fc12a5ec76bfb2cae4e04e03d Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Fri, 26 Jun 2020 08:37:06 -0600 Subject: [PATCH 1/2] SPV: Partially address #2293: correct "const in" precision matching. Track whether formal parameters declare reduced precision and match that with arguments, and if they differ, make a copy to promote the precision. --- SPIRV/GlslangToSpv.cpp | 10 +++- SPIRV/SpvBuilder.cpp | 5 +- SPIRV/spvIR.h | 6 +++ Test/baseResults/spv.precisionArgs.frag.out | 59 +++++++++++++++++++++ Test/spv.precisionArgs.frag | 15 ++++++ gtests/Spv.FromFile.cpp | 1 + 6 files changed, 94 insertions(+), 2 deletions(-) create mode 100755 Test/baseResults/spv.precisionArgs.frag.out create mode 100644 Test/spv.precisionArgs.frag diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index b63d901c..fb36212a 100644 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -5346,13 +5346,21 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg } ++lValueCount; } else { + const bool argIsRelaxedPrecision = TranslatePrecisionDecoration(*argTypes[a]) == + spv::DecorationRelaxedPrecision; // process r-value, which involves a copy for a type mismatch - if (function->getParamType(a) != convertGlslangToSpvType(*argTypes[a])) { + if (function->getParamType(a) != convertGlslangToSpvType(*argTypes[a]) || + argIsRelaxedPrecision != function->isReducedPrecisionParam(a)) + { spv::Id argCopy = builder.createVariable(spv::StorageClassFunction, function->getParamType(a), "arg"); + if (function->isReducedPrecisionParam(a)) + builder.setPrecision(argCopy, spv::DecorationRelaxedPrecision); builder.clearAccessChain(); builder.setAccessChainLValue(argCopy); multiTypeStore(*argTypes[a], rValues[rValueCount]); arg = builder.createLoad(argCopy); + if (function->isReducedPrecisionParam(a)) + builder.setPrecision(arg, spv::DecorationRelaxedPrecision); } else arg = rValues[rValueCount]; ++rValueCount; diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp index 62b7d0eb..f8842245 100644 --- a/SPIRV/SpvBuilder.cpp +++ b/SPIRV/SpvBuilder.cpp @@ -1298,8 +1298,11 @@ Function* Builder::makeFunctionEntry(Decoration precision, Id returnType, const // Set up the precisions setPrecision(function->getId(), precision); for (unsigned p = 0; p < (unsigned)decorations.size(); ++p) { - for (int d = 0; d < (int)decorations[p].size(); ++d) + for (int d = 0; d < (int)decorations[p].size(); ++d) { addDecoration(firstParamId + p, decorations[p][d]); + if (decorations[p][d] == DecorationRelaxedPrecision) + function->addReducedPrecisionParam(p); + } } // CFG diff --git a/SPIRV/spvIR.h b/SPIRV/spvIR.h index 6523035e..6146518a 100755 --- a/SPIRV/spvIR.h +++ b/SPIRV/spvIR.h @@ -55,6 +55,7 @@ #include #include #include +#include namespace spv { @@ -355,6 +356,10 @@ public: void setImplicitThis() { implicitThis = true; } bool hasImplicitThis() const { return implicitThis; } + void addReducedPrecisionParam(int p) { reducedPrecisionParams.insert(p); } + bool isReducedPrecisionParam(int p) const + { return reducedPrecisionParams.find(p) != reducedPrecisionParams.end(); } + void dump(std::vector& out) const { // OpFunction @@ -379,6 +384,7 @@ protected: std::vector parameterInstructions; std::vector blocks; bool implicitThis; // true if this is a member function expecting to be passed a 'this' as the first argument + std::set reducedPrecisionParams; // list of parameter indexes that need a relaxed precision arg }; // diff --git a/Test/baseResults/spv.precisionArgs.frag.out b/Test/baseResults/spv.precisionArgs.frag.out new file mode 100755 index 00000000..3e9eb660 --- /dev/null +++ b/Test/baseResults/spv.precisionArgs.frag.out @@ -0,0 +1,59 @@ +spv.precisionArgs.frag +// 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" + ExecutionMode 4 OriginUpperLeft + Source ESSL 310 + Name 4 "main" + Name 10 "fooConst(f1;f1;" + Name 8 "f" + Name 9 "g" + Name 13 "aM" + Name 15 "bM" + Name 17 "arg" + Name 20 "aH" + Name 22 "bH" + Name 24 "arg" + Decorate 8(f) RelaxedPrecision + Decorate 13(aM) RelaxedPrecision + Decorate 14 RelaxedPrecision + Decorate 15(bM) RelaxedPrecision + Decorate 16 RelaxedPrecision + Decorate 24(arg) RelaxedPrecision + Decorate 25 RelaxedPrecision + 2: TypeVoid + 3: TypeFunction 2 + 6: TypeFloat 32 + 7: TypeFunction 2 6(float) 6(float) + 12: TypePointer Function 6(float) + 4(main): 2 Function None 3 + 5: Label + 13(aM): 12(ptr) Variable Function + 15(bM): 12(ptr) Variable Function + 17(arg): 12(ptr) Variable Function + 20(aH): 12(ptr) Variable Function + 22(bH): 12(ptr) Variable Function + 24(arg): 12(ptr) Variable Function + 14: 6(float) Load 13(aM) + 16: 6(float) Load 15(bM) + Store 17(arg) 16 + 18: 6(float) Load 17(arg) + 19: 2 FunctionCall 10(fooConst(f1;f1;) 14 18 + 21: 6(float) Load 20(aH) + 23: 6(float) Load 22(bH) + Store 24(arg) 21 + 25: 6(float) Load 24(arg) + 26: 2 FunctionCall 10(fooConst(f1;f1;) 25 23 + Return + FunctionEnd +10(fooConst(f1;f1;): 2 Function None 7 + 8(f): 6(float) FunctionParameter + 9(g): 6(float) FunctionParameter + 11: Label + Return + FunctionEnd diff --git a/Test/spv.precisionArgs.frag b/Test/spv.precisionArgs.frag new file mode 100644 index 00000000..4bf49b3b --- /dev/null +++ b/Test/spv.precisionArgs.frag @@ -0,0 +1,15 @@ +#version 310 es + +precision mediump float; + +void fooConst(const in float f, const in highp float g) +{ +} + +void main() +{ + float aM, bM; + highp float aH, bH; + fooConst(aM, bM); // must copy bM + fooConst(aH, bH); // must copy aH +} diff --git a/gtests/Spv.FromFile.cpp b/gtests/Spv.FromFile.cpp index e1d97352..80a75aab 100644 --- a/gtests/Spv.FromFile.cpp +++ b/gtests/Spv.FromFile.cpp @@ -375,6 +375,7 @@ INSTANTIATE_TEST_CASE_P( "spv.Operations.frag", "spv.paramMemory.frag", "spv.precision.frag", + "spv.precisionArgs.frag", "spv.precisionNonESSamp.frag", "spv.prepost.frag", "spv.privateVariableTypes.frag", From bf6efd0316d89bbe7e70c8581cf0b707d0a4af38 Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Fri, 26 Jun 2020 09:05:31 -0600 Subject: [PATCH 2/2] SPV: Fix #2293: keep relaxed precision on arg passed to relaxed param When arguments are copied to make space for a writable formal parameter, and the formal parameter is relaxed precision, make the copy also relaxed precision. --- SPIRV/GlslangToSpv.cpp | 2 + Test/baseResults/spv.forwardFun.frag.out | 1 + .../spv.noDeadDecorations.vert.out | 1 + Test/baseResults/spv.precisionArgs.frag.out | 91 +++++++++++++------ Test/baseResults/spv.switch.frag.out | 6 ++ Test/spv.precisionArgs.frag | 8 +- 6 files changed, 77 insertions(+), 32 deletions(-) diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index fb36212a..e0480d18 100644 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -5336,6 +5336,8 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg // need space to hold the copy arg = builder.createVariable(spv::StorageClassFunction, builder.getContainedTypeId(function->getParamType(a)), "param"); + if (function->isReducedPrecisionParam(a)) + builder.setPrecision(arg, spv::DecorationRelaxedPrecision); if (qualifiers[a] == glslang::EvqIn || qualifiers[a] == glslang::EvqInOut) { // need to copy the input into output space builder.setAccessChain(lValues[lValueCount]); diff --git a/Test/baseResults/spv.forwardFun.frag.out b/Test/baseResults/spv.forwardFun.frag.out index 0a0e5e31..e0d749b0 100644 --- a/Test/baseResults/spv.forwardFun.frag.out +++ b/Test/baseResults/spv.forwardFun.frag.out @@ -26,6 +26,7 @@ spv.forwardFun.frag Decorate 15(bar) RelaxedPrecision Decorate 18(color) RelaxedPrecision Decorate 20(BaseColor) RelaxedPrecision + Decorate 21(param) RelaxedPrecision Decorate 22 RelaxedPrecision Decorate 23 RelaxedPrecision Decorate 24 RelaxedPrecision diff --git a/Test/baseResults/spv.noDeadDecorations.vert.out b/Test/baseResults/spv.noDeadDecorations.vert.out index e48cba02..4a4d7b38 100644 --- a/Test/baseResults/spv.noDeadDecorations.vert.out +++ b/Test/baseResults/spv.noDeadDecorations.vert.out @@ -23,6 +23,7 @@ spv.noDeadDecorations.vert MemberDecorate 20(gl_PerVertex) 0 BuiltIn Position MemberDecorate 20(gl_PerVertex) 1 BuiltIn PointSize Decorate 20(gl_PerVertex) Block + Decorate 26(param) RelaxedPrecision Decorate 27 RelaxedPrecision 2: TypeVoid 3: TypeFunction 2 diff --git a/Test/baseResults/spv.precisionArgs.frag.out b/Test/baseResults/spv.precisionArgs.frag.out index 3e9eb660..b670fc85 100755 --- a/Test/baseResults/spv.precisionArgs.frag.out +++ b/Test/baseResults/spv.precisionArgs.frag.out @@ -1,7 +1,7 @@ spv.precisionArgs.frag // Module Version 10000 // Generated by (magic number): 8000a -// Id's are bound by 27 +// Id's are bound by 42 Capability Shader 1: ExtInstImport "GLSL.std.450" @@ -13,42 +13,69 @@ spv.precisionArgs.frag Name 10 "fooConst(f1;f1;" Name 8 "f" Name 9 "g" - Name 13 "aM" - Name 15 "bM" - Name 17 "arg" - Name 20 "aH" - Name 22 "bH" - Name 24 "arg" + Name 16 "foo(f1;f1;" + Name 14 "f" + Name 15 "g" + Name 18 "aM" + Name 20 "bM" + Name 22 "arg" + Name 25 "aH" + Name 27 "bH" + Name 29 "arg" + Name 32 "param" + Name 34 "param" + Name 37 "param" + Name 39 "param" Decorate 8(f) RelaxedPrecision - Decorate 13(aM) RelaxedPrecision - Decorate 14 RelaxedPrecision - Decorate 15(bM) RelaxedPrecision - Decorate 16 RelaxedPrecision - Decorate 24(arg) RelaxedPrecision - Decorate 25 RelaxedPrecision + Decorate 14(f) RelaxedPrecision + Decorate 18(aM) RelaxedPrecision + Decorate 19 RelaxedPrecision + Decorate 20(bM) RelaxedPrecision + Decorate 21 RelaxedPrecision + Decorate 29(arg) RelaxedPrecision + Decorate 30 RelaxedPrecision + Decorate 32(param) RelaxedPrecision + Decorate 33 RelaxedPrecision + Decorate 35 RelaxedPrecision + Decorate 37(param) RelaxedPrecision 2: TypeVoid 3: TypeFunction 2 6: TypeFloat 32 7: TypeFunction 2 6(float) 6(float) 12: TypePointer Function 6(float) + 13: TypeFunction 2 12(ptr) 12(ptr) 4(main): 2 Function None 3 5: Label - 13(aM): 12(ptr) Variable Function - 15(bM): 12(ptr) Variable Function - 17(arg): 12(ptr) Variable Function - 20(aH): 12(ptr) Variable Function - 22(bH): 12(ptr) Variable Function - 24(arg): 12(ptr) Variable Function - 14: 6(float) Load 13(aM) - 16: 6(float) Load 15(bM) - Store 17(arg) 16 - 18: 6(float) Load 17(arg) - 19: 2 FunctionCall 10(fooConst(f1;f1;) 14 18 - 21: 6(float) Load 20(aH) - 23: 6(float) Load 22(bH) - Store 24(arg) 21 - 25: 6(float) Load 24(arg) - 26: 2 FunctionCall 10(fooConst(f1;f1;) 25 23 + 18(aM): 12(ptr) Variable Function + 20(bM): 12(ptr) Variable Function + 22(arg): 12(ptr) Variable Function + 25(aH): 12(ptr) Variable Function + 27(bH): 12(ptr) Variable Function + 29(arg): 12(ptr) Variable Function + 32(param): 12(ptr) Variable Function + 34(param): 12(ptr) Variable Function + 37(param): 12(ptr) Variable Function + 39(param): 12(ptr) Variable Function + 19: 6(float) Load 18(aM) + 21: 6(float) Load 20(bM) + Store 22(arg) 21 + 23: 6(float) Load 22(arg) + 24: 2 FunctionCall 10(fooConst(f1;f1;) 19 23 + 26: 6(float) Load 25(aH) + 28: 6(float) Load 27(bH) + Store 29(arg) 26 + 30: 6(float) Load 29(arg) + 31: 2 FunctionCall 10(fooConst(f1;f1;) 30 28 + 33: 6(float) Load 18(aM) + Store 32(param) 33 + 35: 6(float) Load 20(bM) + Store 34(param) 35 + 36: 2 FunctionCall 16(foo(f1;f1;) 32(param) 34(param) + 38: 6(float) Load 25(aH) + Store 37(param) 38 + 40: 6(float) Load 27(bH) + Store 39(param) 40 + 41: 2 FunctionCall 16(foo(f1;f1;) 37(param) 39(param) Return FunctionEnd 10(fooConst(f1;f1;): 2 Function None 7 @@ -57,3 +84,9 @@ spv.precisionArgs.frag 11: Label Return FunctionEnd + 16(foo(f1;f1;): 2 Function None 13 + 14(f): 12(ptr) FunctionParameter + 15(g): 12(ptr) FunctionParameter + 17: Label + Return + FunctionEnd diff --git a/Test/baseResults/spv.switch.frag.out b/Test/baseResults/spv.switch.frag.out index dda061f3..cd6dde26 100644 --- a/Test/baseResults/spv.switch.frag.out +++ b/Test/baseResults/spv.switch.frag.out @@ -154,15 +154,21 @@ WARNING: 0:139: 'switch' : last case/default label not followed by statements Decorate 230 RelaxedPrecision Decorate 231 RelaxedPrecision Decorate 233(v) RelaxedPrecision + Decorate 234(param) RelaxedPrecision Decorate 235 RelaxedPrecision + Decorate 236(param) RelaxedPrecision Decorate 237 RelaxedPrecision + Decorate 238(param) RelaxedPrecision Decorate 239 RelaxedPrecision Decorate 240 RelaxedPrecision Decorate 243 RelaxedPrecision Decorate 244 RelaxedPrecision Decorate 245 RelaxedPrecision + Decorate 246(param) RelaxedPrecision Decorate 247 RelaxedPrecision + Decorate 248(param) RelaxedPrecision Decorate 249 RelaxedPrecision + Decorate 250(param) RelaxedPrecision Decorate 251 RelaxedPrecision Decorate 252 RelaxedPrecision Decorate 254 RelaxedPrecision diff --git a/Test/spv.precisionArgs.frag b/Test/spv.precisionArgs.frag index 4bf49b3b..24be1d07 100644 --- a/Test/spv.precisionArgs.frag +++ b/Test/spv.precisionArgs.frag @@ -2,9 +2,9 @@ precision mediump float; -void fooConst(const in float f, const in highp float g) -{ -} +void fooConst(const in float f, const in highp float g) { } + +void foo(in float f, in highp float g) { } void main() { @@ -12,4 +12,6 @@ void main() highp float aH, bH; fooConst(aM, bM); // must copy bM fooConst(aH, bH); // must copy aH + foo(aM, bM); + foo(aH, bH); }