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
diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp
index e0480d1..87a2830 100644
--- a/SPIRV/GlslangToSpv.cpp
+++ b/SPIRV/GlslangToSpv.cpp
@@ -2981,7 +2981,8 @@
// 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 @@
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 @@
} 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 @@
// 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 @@
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 @@
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 @@
++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 @@
}
++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 @@
++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);
}
diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp
index f884224..fba26ad 100644
--- a/SPIRV/SpvBuilder.cpp
+++ b/SPIRV/SpvBuilder.cpp
@@ -1297,11 +1297,11 @@
// 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 @@
}
// 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 @@
if (name)
addName(inst->getResultId(), name);
+ setPrecision(inst->getResultId(), precision);
return inst->getResultId();
}
@@ -1437,7 +1438,8 @@
}
// 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 @@
}
buildPoint->addInstruction(std::unique_ptr<Instruction>(load));
+ setPrecision(load->getResultId(), precision);
return load->getResultId();
}
@@ -2677,7 +2680,7 @@
// 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 @@
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 @@
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 @@
// 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);
}
diff --git a/SPIRV/SpvBuilder.h b/SPIRV/SpvBuilder.h
index 71b90d6..ca126d3 100644
--- a/SPIRV/SpvBuilder.h
+++ b/SPIRV/SpvBuilder.h
@@ -347,7 +347,8 @@
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 @@
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
diff --git a/SPIRV/spvIR.h b/SPIRV/spvIR.h
index 6146518..868b9bf 100755
--- a/SPIRV/spvIR.h
+++ b/SPIRV/spvIR.h
@@ -352,13 +352,27 @@
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 @@
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 @@
// - 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);