Skip to content

G
glslang
Commit 435dd802 by John Kessenich
Options

SPV: RelaxedPrecision: Generalize fix #2293 to cover more operations.

This simplifies and enforces use of precision in many more places, to help avoid accidental loss of RelaxedPrecision through intermediate operations. Known fixes are: - ?: - function return values with mis-matched precision - precision of function return values when a copy was needed to fix types
parent 27e915ed
Showing with 225 additions and 113 deletions
SPIRV/GlslangToSpv.cpp
......@@ -2981,7 +2981,8 @@ bool TGlslangToSpvTraverser::visitAggregate(glslang::TVisit visit, glslang::TInt
// receive the result, and must later swizzle that into the original
// l-value.
complexLvalues.push_back(builder.getAccessChain());
temporaryLvalues.push_back(builder.createVariable(spv::StorageClassFunction,
temporaryLvalues.push_back(builder.createVariable(
spv::NoPrecision, spv::StorageClassFunction,
builder.accessChainGetInferredType(), "swizzleTemp"));
operands.push_back(temporaryLvalues.back());
} else {
......@@ -3084,7 +3085,7 @@ bool TGlslangToSpvTraverser::visitAggregate(glslang::TVisit visit, glslang::TInt
for (unsigned int i = 0; i < temporaryLvalues.size(); ++i) {
builder.setAccessChain(complexLvalues[i]);
builder.accessChainStore(builder.createLoad(temporaryLvalues[i]));
builder.accessChainStore(builder.createLoad(temporaryLvalues[i], spv::NoPrecision));
}
}
......@@ -3201,7 +3202,8 @@ bool TGlslangToSpvTraverser::visitSelection(glslang::TVisit /* visit */, glslang
} else {
// We need control flow to select the result.
// TODO: Once SPIR-V OpSelect allows arbitrary types, eliminate this path.
result = builder.createVariable(spv::StorageClassFunction, convertGlslangToSpvType(node->getType()));
result = builder.createVariable(TranslatePrecisionDecoration(node->getType()),
spv::StorageClassFunction, convertGlslangToSpvType(node->getType()));
// Selection control:
const spv::SelectionControlMask control = TranslateSelectionControl(*node);
......@@ -3226,8 +3228,10 @@ bool TGlslangToSpvTraverser::visitSelection(glslang::TVisit /* visit */, glslang
// Execute the one side needed, as per the condition
const auto executeOneSide = [&]() {
// Always emit control flow.
if (node->getBasicType() != glslang::EbtVoid)
result = builder.createVariable(spv::StorageClassFunction, convertGlslangToSpvType(node->getType()));
if (node->getBasicType() != glslang::EbtVoid) {
result = builder.createVariable(TranslatePrecisionDecoration(node->getType()), spv::StorageClassFunction,
convertGlslangToSpvType(node->getType()));
}
// Selection control:
const spv::SelectionControlMask control = TranslateSelectionControl(*node);
......@@ -3424,15 +3428,17 @@ bool TGlslangToSpvTraverser::visitBranch(glslang::TVisit /* visit */, glslang::T
builder.createLoopContinue();
break;
case glslang::EOpReturn:
if (node->getExpression()) {
if (node->getExpression() != nullptr) {
const glslang::TType& glslangReturnType = node->getExpression()->getType();
spv::Id returnId = accessChainLoad(glslangReturnType);
if (builder.getTypeId(returnId) != currentFunction->getReturnType()) {
if (builder.getTypeId(returnId) != currentFunction->getReturnType() ||
TranslatePrecisionDecoration(glslangReturnType) != currentFunction->getReturnPrecision()) {
builder.clearAccessChain();
spv::Id copyId = builder.createVariable(spv::StorageClassFunction, currentFunction->getReturnType());
spv::Id copyId = builder.createVariable(currentFunction->getReturnPrecision(),
spv::StorageClassFunction, currentFunction->getReturnType());
builder.setAccessChainLValue(copyId);
multiTypeStore(glslangReturnType, returnId);
returnId = builder.createLoad(copyId);
returnId = builder.createLoad(copyId, currentFunction->getReturnPrecision());
}
builder.makeReturn(false, returnId);
} else
......@@ -3539,7 +3545,7 @@ spv::Id TGlslangToSpvTraverser::createSpvVariable(const glslang::TIntermSymbol*
false /* specConst */);
}
return builder.createVariable(storageClass, spvType, name, initializer);
return builder.createVariable(spv::NoPrecision, storageClass, spvType, name, initializer);
}
// Return type Id of the sampled type.
......@@ -5334,10 +5340,8 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg
++lValueCount;
} else if (writableParam(qualifiers[a])) {
// need space to hold the copy
arg = builder.createVariable(spv::StorageClassFunction,
arg = builder.createVariable(function->getParamPrecision(a), 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]);
......@@ -5348,21 +5352,15 @@ 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]) ||
argIsRelaxedPrecision != function->isReducedPrecisionParam(a))
TranslatePrecisionDecoration(*argTypes[a]) != function->getParamPrecision(a))
{
spv::Id argCopy = builder.createVariable(spv::StorageClassFunction, function->getParamType(a), "arg");
if (function->isReducedPrecisionParam(a))
builder.setPrecision(argCopy, spv::DecorationRelaxedPrecision);
spv::Id argCopy = builder.createVariable(function->getParamPrecision(a), spv::StorageClassFunction, function->getParamType(a), "arg");
builder.clearAccessChain();
builder.setAccessChainLValue(argCopy);
multiTypeStore(*argTypes[a], rValues[rValueCount]);
arg = builder.createLoad(argCopy);
if (function->isReducedPrecisionParam(a))
builder.setPrecision(arg, spv::DecorationRelaxedPrecision);
arg = builder.createLoad(argCopy, function->getParamPrecision(a));
} else
arg = rValues[rValueCount];
++rValueCount;
......@@ -5381,7 +5379,7 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg
++lValueCount;
else if (writableParam(qualifiers[a])) {
if (qualifiers[a] == glslang::EvqOut || qualifiers[a] == glslang::EvqInOut) {
spv::Id copy = builder.createLoad(spvArgs[a]);
spv::Id copy = builder.createLoad(spvArgs[a], spv::NoPrecision);
builder.setAccessChain(lValues[lValueCount]);
multiTypeStore(*argTypes[a], copy);
}
......
SPIRV/SpvBuilder.cpp
......@@ -1297,11 +1297,11 @@ Function* Builder::makeFunctionEntry(Decoration precision, Id returnType, const
// Set up the precisions
setPrecision(function->getId(), precision);
function->setReturnPrecision(precision);
for (unsigned p = 0; p < (unsigned)decorations.size(); ++p) {
for (int d = 0; d < (int)decorations[p].size(); ++d) {
addDecoration(firstParamId + p, decorations[p][d]);
if (decorations[p][d] == DecorationRelaxedPrecision)
function->addReducedPrecisionParam(p);
function->addParamPrecision(p, decorations[p][d]);
}
}
......@@ -1359,7 +1359,7 @@ void Builder::makeDiscard()
}
// Comments in header
Id Builder::createVariable(StorageClass storageClass, Id type, const char* name, Id initializer)
Id Builder::createVariable(Decoration precision, StorageClass storageClass, Id type, const char* name, Id initializer)
{
Id pointerType = makePointer(storageClass, type);
Instruction* inst = new Instruction(getUniqueId(), pointerType, OpVariable);
......@@ -1381,6 +1381,7 @@ Id Builder::createVariable(StorageClass storageClass, Id type, const char* name,
if (name)
addName(inst->getResultId(), name);
setPrecision(inst->getResultId(), precision);
return inst->getResultId();
}
......@@ -1437,7 +1438,8 @@ void Builder::createStore(Id rValue, Id lValue, spv::MemoryAccessMask memoryAcce
}
// Comments in header
Id Builder::createLoad(Id lValue, spv::MemoryAccessMask memoryAccess, spv::Scope scope, unsigned int alignment)
Id Builder::createLoad(Id lValue, spv::Decoration precision, spv::MemoryAccessMask memoryAccess,
spv::Scope scope, unsigned int alignment)
{
Instruction* load = new Instruction(getUniqueId(), getDerefTypeId(lValue), OpLoad);
load->addIdOperand(lValue);
......@@ -1455,6 +1457,7 @@ Id Builder::createLoad(Id lValue, spv::MemoryAccessMask memoryAccess, spv::Scope
}
buildPoint->addInstruction(std::unique_ptr<Instruction>(load));
setPrecision(load->getResultId(), precision);
return load->getResultId();
}
......@@ -2677,7 +2680,7 @@ void Builder::accessChainStore(Id rvalue, spv::MemoryAccessMask memoryAccess, sp
// If swizzle still exists, it is out-of-order or not full, we must load the target vector,
// extract and insert elements to perform writeMask and/or swizzle.
if (accessChain.swizzle.size() > 0) {
Id tempBaseId = createLoad(base);
Id tempBaseId = createLoad(base, spv::NoPrecision);
source = createLvalueSwizzle(getTypeId(tempBaseId), tempBaseId, source, accessChain.swizzle);
}
......@@ -2716,17 +2719,17 @@ Id Builder::accessChainLoad(Decoration precision, Decoration nonUniform, Id resu
if (constant) {
id = createCompositeExtract(accessChain.base, swizzleBase, indexes);
setPrecision(id, precision);
} else {
Id lValue = NoResult;
if (spvVersion >= Spv_1_4) {
// make a new function variable for this r-value, using an initializer,
// and mark it as NonWritable so that downstream it can be detected as a lookup
// table
lValue = createVariable(StorageClassFunction, getTypeId(accessChain.base), "indexable",
accessChain.base);
lValue = createVariable(NoPrecision, StorageClassFunction, getTypeId(accessChain.base), "indexable", accessChain.base);
addDecoration(lValue, DecorationNonWritable);
} else {
lValue = createVariable(StorageClassFunction, getTypeId(accessChain.base), "indexable");
lValue = createVariable(NoPrecision, StorageClassFunction, getTypeId(accessChain.base), "indexable");
// store into it
createStore(accessChain.base, lValue);
}
......@@ -2735,9 +2738,8 @@ Id Builder::accessChainLoad(Decoration precision, Decoration nonUniform, Id resu
accessChain.isRValue = false;
// load through the access chain
id = createLoad(collapseAccessChain());
id = createLoad(collapseAccessChain(), precision);
}
setPrecision(id, precision);
} else
id = accessChain.base; // no precision, it was set when this was defined
} else {
......@@ -2756,8 +2758,7 @@ Id Builder::accessChainLoad(Decoration precision, Decoration nonUniform, Id resu
// loaded image types get decorated. TODO: This should maybe move to
// createImageTextureFunctionCall.
addDecoration(id, nonUniform);
id = createLoad(id, memoryAccess, scope, alignment);
setPrecision(id, precision);
id = createLoad(id, precision, memoryAccess, scope, alignment);
addDecoration(id, nonUniform);
}
......
SPIRV/SpvBuilder.h
......@@ -347,7 +347,8 @@ public:
void makeDiscard();
// Create a global or function local or IO variable.
Id createVariable(StorageClass, Id type, const char* name = 0, Id initializer = NoResult);
Id createVariable(Decoration precision, StorageClass, Id type, const char* name = nullptr,
Id initializer = NoResult);
// Create an intermediate with an undefined value.
Id createUndefined(Id type);
......@@ -357,7 +358,8 @@ public:
spv::Scope scope = spv::ScopeMax, unsigned int alignment = 0);
// Load from an Id and return it
Id createLoad(Id lValue, spv::MemoryAccessMask memoryAccess = spv::MemoryAccessMaskNone,
Id createLoad(Id lValue, spv::Decoration precision,
spv::MemoryAccessMask memoryAccess = spv::MemoryAccessMaskNone,
spv::Scope scope = spv::ScopeMax, unsigned int alignment = 0);
// Create an OpAccessChain instruction
......
SPIRV/spvIR.h
......@@ -352,13 +352,27 @@ public:
const std::vector<Block*>& getBlocks() const { return blocks; }
void addLocalVariable(std::unique_ptr<Instruction> inst);
Id getReturnType() const { return functionInstruction.getTypeId(); }
void setReturnPrecision(Decoration precision)
{
if (precision == DecorationRelaxedPrecision)
reducedPrecisionReturn = true;
}
Decoration getReturnPrecision() const
{ return reducedPrecisionReturn ? DecorationRelaxedPrecision : NoPrecision; }
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 addParamPrecision(unsigned param, Decoration precision)
{
if (precision == DecorationRelaxedPrecision)
reducedPrecisionParams.insert(param);
}
Decoration getParamPrecision(unsigned param) const
{
return reducedPrecisionParams.find(param) != reducedPrecisionParams.end() ?
DecorationRelaxedPrecision : NoPrecision;
}
void dump(std::vector<unsigned int>& out) const
{
......@@ -384,6 +398,7 @@ protected:
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
bool reducedPrecisionReturn;
std::set<int> reducedPrecisionParams; // list of parameter indexes that need a relaxed precision arg
};
......@@ -445,7 +460,8 @@ protected:
// - the OpFunction instruction
// - all the OpFunctionParameter instructions
__inline Function::Function(Id id, Id resultType, Id functionType, Id firstParamId, Module& parent)
: parent(parent), functionInstruction(id, resultType, OpFunction), implicitThis(false)
: parent(parent), functionInstruction(id, resultType, OpFunction), implicitThis(false),
reducedPrecisionReturn(false)
{
// OpFunction
functionInstruction.addImmediateOperand(FunctionControlMaskNone);
......
Test/baseResults/spv.forwardFun.frag.out
spv.forwardFun.frag
// Module Version 10000
// Generated by (magic number): 8000a
// Id's are bound by 60
// Id's are bound by 64
Capability Shader
1: ExtInstImport "GLSL.std.450"
MemoryModel Logical GLSL450
EntryPoint Fragment 4 "main" 20 30 36 59
EntryPoint Fragment 4 "main" 20 30 36 63
ExecutionMode 4 OriginUpperLeft
Source GLSL 140
Name 4 "main"
......@@ -20,7 +20,7 @@ spv.forwardFun.frag
Name 27 "f"
Name 30 "gl_FragColor"
Name 36 "d"
Name 59 "bigColor"
Name 63 "bigColor"
Decorate 10(unreachableReturn() RelaxedPrecision
Decorate 16(foo(vf4;) RelaxedPrecision
Decorate 15(bar) RelaxedPrecision
......@@ -39,10 +39,14 @@ spv.forwardFun.frag
Decorate 33 RelaxedPrecision
Decorate 36(d) RelaxedPrecision
Decorate 37 RelaxedPrecision
Decorate 52 RelaxedPrecision
Decorate 55 RelaxedPrecision
Decorate 44 RelaxedPrecision
Decorate 45 RelaxedPrecision
Decorate 49 RelaxedPrecision
Decorate 50 RelaxedPrecision
Decorate 56 RelaxedPrecision
Decorate 59(bigColor) RelaxedPrecision
Decorate 59 RelaxedPrecision
Decorate 60 RelaxedPrecision
Decorate 63(bigColor) RelaxedPrecision
2: TypeVoid
3: TypeFunction 2
8: TypeFloat 32
......@@ -60,11 +64,11 @@ spv.forwardFun.frag
38: 8(float) Constant 1082549862
39: TypeBool
43: 8(float) Constant 1067030938
46: 8(float) Constant 1083179008
49: TypeInt 32 0
50: 49(int) Constant 0
53: 49(int) Constant 1
59(bigColor): 19(ptr) Variable Input
48: 8(float) Constant 1083179008
53: TypeInt 32 0
54: 53(int) Constant 0
57: 53(int) Constant 1
63(bigColor): 19(ptr) Variable Input
4(main): 2 Function None 3
5: Label
18(color): 13(ptr) Variable Function
......@@ -90,25 +94,31 @@ spv.forwardFun.frag
FunctionEnd
10(unreachableReturn(): 8(float) Function None 9
11: Label
44: 26(ptr) Variable Function
49: 26(ptr) Variable Function
34: 2 FunctionCall 6(bar()
37: 8(float) Load 36(d)
40: 39(bool) FOrdLessThan 37 38
SelectionMerge 42 None
BranchConditional 40 41 45
BranchConditional 40 41 47
41: Label
ReturnValue 43
45: Label
ReturnValue 46
Store 44 43
45: 8(float) Load 44
ReturnValue 45
47: Label
Store 49 48
50: 8(float) Load 49
ReturnValue 50
42: Label
Unreachable
FunctionEnd
16(foo(vf4;): 8(float) Function None 14
15(bar): 13(ptr) FunctionParameter
17: Label
51: 26(ptr) AccessChain 15(bar) 50
52: 8(float) Load 51
54: 26(ptr) AccessChain 15(bar) 53
55: 8(float) Load 54
56: 8(float) FAdd 52 55
ReturnValue 56
55: 26(ptr) AccessChain 15(bar) 54
56: 8(float) Load 55
58: 26(ptr) AccessChain 15(bar) 57
59: 8(float) Load 58
60: 8(float) FAdd 56 59
ReturnValue 60
FunctionEnd
Test/spv.precision.frag
......@@ -57,4 +57,6 @@ void main()
mediumfout *= s.a;
mediumfout *= s.b;
mediumfout = ((mediumfin * mediumfin > 4.2) ? 2.0 * mediumfout : 3.0 * mediumfout);
}
Test/spv.precisionArgs.frag
......@@ -6,6 +6,11 @@ void fooConst(const in float f, const in highp float g) { }
void foo(in float f, in highp float g) { }
float retM ( float x) { return x; }
highp float retH (highp float x) { return x; }
float retHM(highp float x) { return x; }
highp float retMH( float x) { return x; }
void main()
{
float aM, bM;
......@@ -14,4 +19,9 @@ void main()
fooConst(aH, bH); // must copy aH
foo(aM, bM);
foo(aH, bH);
retM(aM);
retH(aH);
retHM(aH);
retMH(aM);
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment