HLSL: Fix #1249: Always execute both sides of ternary "?:".
This is semantically required by HLSL, and frequently results in using OpSelect instead of control flow.
This commit is contained in:
John Kessenich
@@ -1973,18 +1973,29 @@ bool TGlslangToSpvTraverser::visitAggregate(glslang::TVisit visit, glslang::TInt
|
||||
// next layer copies r-values into memory to use the access-chain mechanism
|
||||
bool TGlslangToSpvTraverser::visitSelection(glslang::TVisit /* visit */, glslang::TIntermSelection* node)
|
||||
{
|
||||
// See if it simple and safe to generate OpSelect instead of using control flow.
|
||||
// Crucially, side effects must be avoided, and there are performance trade-offs.
|
||||
// Return true if good idea (and safe) for OpSelect, false otherwise.
|
||||
const auto selectPolicy = [&]() -> bool {
|
||||
if ((!node->getType().isScalar() && !node->getType().isVector()) ||
|
||||
node->getBasicType() == glslang::EbtVoid)
|
||||
return false;
|
||||
|
||||
// See if it simple and safe, or required, to execute both sides.
|
||||
// Crucially, side effects must be either semantically required or avoided,
|
||||
// and there are performance trade-offs.
|
||||
// Return true if required or a good idea (and safe) to execute both sides,
|
||||
// false otherwise.
|
||||
const auto bothSidesPolicy = [&]() -> bool {
|
||||
// do we have both sides?
|
||||
if (node->getTrueBlock() == nullptr ||
|
||||
node->getFalseBlock() == nullptr)
|
||||
return false;
|
||||
|
||||
// required? (unless we write additional code to look for side effects
|
||||
// and make performance trade-offs if none are present)
|
||||
if (!node->getShortCircuit())
|
||||
return true;
|
||||
|
||||
// if not required to execute both, decide based on performance/practicality...
|
||||
|
||||
// see if OpSelect can handle it
|
||||
if ((!node->getType().isScalar() && !node->getType().isVector()) ||
|
||||
node->getBasicType() == glslang::EbtVoid)
|
||||
return false;
|
||||
|
||||
assert(node->getType() == node->getTrueBlock() ->getAsTyped()->getType() &&
|
||||
node->getType() == node->getFalseBlock()->getAsTyped()->getType());
|
||||
|
||||
@@ -1997,10 +2008,14 @@ bool TGlslangToSpvTraverser::visitSelection(glslang::TVisit /* visit */, glslang
|
||||
operandOkay(node->getFalseBlock()->getAsTyped());
|
||||
};
|
||||
|
||||
// Emit OpSelect for this selection.
|
||||
const auto handleAsOpSelect = [&]() {
|
||||
node->getCondition()->traverse(this);
|
||||
spv::Id condition = accessChainLoad(node->getCondition()->getType());
|
||||
spv::Id result = spv::NoResult; // upcoming result selecting between trueValue and falseValue
|
||||
// emit the condition before doing anything with selection
|
||||
node->getCondition()->traverse(this);
|
||||
spv::Id condition = accessChainLoad(node->getCondition()->getType());
|
||||
|
||||
// Find a way of executing both sides and selecting the right result.
|
||||
const auto executeBothSides = [&]() -> void {
|
||||
// execute both sides
|
||||
node->getTrueBlock()->traverse(this);
|
||||
spv::Id trueValue = accessChainLoad(node->getTrueBlock()->getAsTyped()->getType());
|
||||
node->getFalseBlock()->traverse(this);
|
||||
@@ -2008,72 +2023,98 @@ bool TGlslangToSpvTraverser::visitSelection(glslang::TVisit /* visit */, glslang
|
||||
|
||||
builder.setLine(node->getLoc().line);
|
||||
|
||||
// smear condition to vector, if necessary (AST is always scalar)
|
||||
if (builder.isVector(trueValue))
|
||||
condition = builder.smearScalar(spv::NoPrecision, condition,
|
||||
builder.makeVectorType(builder.makeBoolType(),
|
||||
builder.getNumComponents(trueValue)));
|
||||
// done if void
|
||||
if (node->getBasicType() == glslang::EbtVoid)
|
||||
return;
|
||||
|
||||
spv::Id select = builder.createTriOp(spv::OpSelect,
|
||||
convertGlslangToSpvType(node->getType()), condition,
|
||||
trueValue, falseValue);
|
||||
builder.clearAccessChain();
|
||||
builder.setAccessChainRValue(select);
|
||||
// emit code to select between trueValue and falseValue
|
||||
|
||||
// see if OpSelect can handle it
|
||||
if (node->getType().isScalar() || node->getType().isVector()) {
|
||||
// Emit OpSelect for this selection.
|
||||
|
||||
// smear condition to vector, if necessary (AST is always scalar)
|
||||
if (builder.isVector(trueValue))
|
||||
condition = builder.smearScalar(spv::NoPrecision, condition,
|
||||
builder.makeVectorType(builder.makeBoolType(),
|
||||
builder.getNumComponents(trueValue)));
|
||||
|
||||
// OpSelect
|
||||
result = builder.createTriOp(spv::OpSelect,
|
||||
convertGlslangToSpvType(node->getType()), condition,
|
||||
trueValue, falseValue);
|
||||
|
||||
builder.clearAccessChain();
|
||||
builder.setAccessChainRValue(result);
|
||||
} 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()));
|
||||
|
||||
// Selection control:
|
||||
const spv::SelectionControlMask control = TranslateSelectionControl(*node);
|
||||
|
||||
// make an "if" based on the value created by the condition
|
||||
spv::Builder::If ifBuilder(condition, control, builder);
|
||||
|
||||
// emit the "then" statement
|
||||
builder.createStore(trueValue, result);
|
||||
ifBuilder.makeBeginElse();
|
||||
// emit the "else" statement
|
||||
builder.createStore(falseValue, result);
|
||||
|
||||
// finish off the control flow
|
||||
ifBuilder.makeEndIf();
|
||||
|
||||
builder.clearAccessChain();
|
||||
builder.setAccessChainLValue(result);
|
||||
}
|
||||
};
|
||||
|
||||
// Try for OpSelect
|
||||
// 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 (selectPolicy()) {
|
||||
// Selection control:
|
||||
const spv::SelectionControlMask control = TranslateSelectionControl(*node);
|
||||
|
||||
// make an "if" based on the value created by the condition
|
||||
spv::Builder::If ifBuilder(condition, control, builder);
|
||||
|
||||
// emit the "then" statement
|
||||
if (node->getTrueBlock() != nullptr) {
|
||||
node->getTrueBlock()->traverse(this);
|
||||
if (result != spv::NoResult)
|
||||
builder.createStore(accessChainLoad(node->getTrueBlock()->getAsTyped()->getType()), result);
|
||||
}
|
||||
|
||||
if (node->getFalseBlock() != nullptr) {
|
||||
ifBuilder.makeBeginElse();
|
||||
// emit the "else" statement
|
||||
node->getFalseBlock()->traverse(this);
|
||||
if (result != spv::NoResult)
|
||||
builder.createStore(accessChainLoad(node->getFalseBlock()->getAsTyped()->getType()), result);
|
||||
}
|
||||
|
||||
// finish off the control flow
|
||||
ifBuilder.makeEndIf();
|
||||
|
||||
if (result != spv::NoResult) {
|
||||
builder.clearAccessChain();
|
||||
builder.setAccessChainLValue(result);
|
||||
}
|
||||
};
|
||||
|
||||
// Try for OpSelect (or a requirement to execute both sides)
|
||||
if (bothSidesPolicy()) {
|
||||
SpecConstantOpModeGuard spec_constant_op_mode_setter(&builder);
|
||||
if (node->getType().getQualifier().isSpecConstant())
|
||||
spec_constant_op_mode_setter.turnOnSpecConstantOpMode();
|
||||
|
||||
handleAsOpSelect();
|
||||
return false;
|
||||
}
|
||||
|
||||
// Instead, emit control flow...
|
||||
// Don't handle results as temporaries, because there will be two names
|
||||
// and better to leave SSA to later passes.
|
||||
spv::Id result = (node->getBasicType() == glslang::EbtVoid)
|
||||
? spv::NoResult
|
||||
: builder.createVariable(spv::StorageClassFunction, convertGlslangToSpvType(node->getType()));
|
||||
|
||||
// emit the condition before doing anything with selection
|
||||
node->getCondition()->traverse(this);
|
||||
|
||||
// Selection control:
|
||||
const spv::SelectionControlMask control = TranslateSelectionControl(*node);
|
||||
|
||||
// make an "if" based on the value created by the condition
|
||||
spv::Builder::If ifBuilder(accessChainLoad(node->getCondition()->getType()), control, builder);
|
||||
|
||||
// emit the "then" statement
|
||||
if (node->getTrueBlock() != nullptr) {
|
||||
node->getTrueBlock()->traverse(this);
|
||||
if (result != spv::NoResult)
|
||||
builder.createStore(accessChainLoad(node->getTrueBlock()->getAsTyped()->getType()), result);
|
||||
}
|
||||
|
||||
if (node->getFalseBlock() != nullptr) {
|
||||
ifBuilder.makeBeginElse();
|
||||
// emit the "else" statement
|
||||
node->getFalseBlock()->traverse(this);
|
||||
if (result != spv::NoResult)
|
||||
builder.createStore(accessChainLoad(node->getFalseBlock()->getAsTyped()->getType()), result);
|
||||
}
|
||||
|
||||
// finish off the control flow
|
||||
ifBuilder.makeEndIf();
|
||||
|
||||
if (result != spv::NoResult) {
|
||||
// GLSL only has r-values as the result of a :?, but
|
||||
// if we have an l-value, that can be more efficient if it will
|
||||
// become the base of a complex r-value expression, because the
|
||||
// next layer copies r-values into memory to use the access-chain mechanism
|
||||
builder.clearAccessChain();
|
||||
builder.setAccessChainLValue(result);
|
||||
}
|
||||
executeBothSides();
|
||||
} else
|
||||
executeOneSide();
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user