Skip to content

Commit

Permalink
SPV: Partially address #2293: correct "const in" precision matching.
Browse files Browse the repository at this point in the history
Track whether formal parameters declare reduced precision and match
that with arguments, and if they differ, make a copy to promote the
precision.
  • Loading branch information
johnkslang committed Jun 26, 2020
1 parent fbb9dc2 commit 4df1033
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 2 deletions.
10 changes: 9 additions & 1 deletion SPIRV/GlslangToSpv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion SPIRV/SpvBuilder.cpp
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
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
59 changes: 59 additions & 0 deletions Test/baseResults/spv.precisionArgs.frag.out
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions Test/spv.precisionArgs.frag
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions gtests/Spv.FromFile.cpp
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 4df1033

Please sign in to comment.