Skip to content

Commit

Permalink
Merge pull request #2294 from KhronosGroup/fix-arg-precision
Browse files Browse the repository at this point in the history 
Fix #2293: Get correct RelaxedPrecision settings for various ways of passing arguments to formal parameters.
  • Loading branch information
@johnkslang
johnkslang authored Jun 28, 2020
2 parents fbb9dc2 + bf6efd0 commit be06c6f
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 2 deletions.
12 changes: 11 additions & 1 deletion SPIRV/GlslangToSpv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -5346,13 +5348,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;
Expand Down
5 changes: 4 additions & 1 deletion SPIRV/SpvBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions SPIRV/spvIR.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include <iostream>
#include <memory>
#include <vector>
#include <set>

namespace spv {

Expand Down Expand Up @@ -355,6 +356,10 @@ class Function {
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<unsigned int>& out) const
{
// OpFunction
Expand All @@ -379,6 +384,7 @@ class Function {
std::vector<Instruction*> parameterInstructions;
std::vector<Block*> blocks;
bool implicitThis; // true if this is a member function expecting to be passed a 'this' as the first argument
std::set<int> reducedPrecisionParams; // list of parameter indexes that need a relaxed precision arg
};

//
Expand Down
1 change: 1 addition & 0 deletions Test/baseResults/spv.forwardFun.frag.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions Test/baseResults/spv.noDeadDecorations.vert.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
92 changes: 92 additions & 0 deletions Test/baseResults/spv.precisionArgs.frag.out
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
spv.precisionArgs.frag
// Module Version 10000
// 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"
ExecutionMode 4 OriginUpperLeft
Source ESSL 310
Name 4 "main"
Name 10 "fooConst(f1;f1;"
Name 8 "f"
Name 9 "g"
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 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
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
8(f): 6(float) FunctionParameter
9(g): 6(float) FunctionParameter
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
6 changes: 6 additions & 0 deletions Test/baseResults/spv.switch.frag.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions Test/spv.precisionArgs.frag
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#version 310 es

precision mediump float;

void fooConst(const in float f, const in highp float g) { }

void foo(in float f, 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
foo(aM, bM);
foo(aH, bH);
}
1 change: 1 addition & 0 deletions gtests/Spv.FromFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit be06c6f

Please sign in to comment.