AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Demonstrate a bug with inlined static switches.

Browse Source 
When we detect a static switch, the optimizer finds the matching switch-
case and eliminates all the other switch-cases. It handles case
fall-through by scanning forward and looking for an unconditional break.

However, the inliner has an interesting quirk--it can replace `return`
statements inside of a switch with `continue` statements, since the body
of the inlined function has been wrapped with a for-loop to allow for
early exits. The optimizer does not recognize these continue statements
as exits from the switch (although they certainly qualify), so it
treats continues as fallen-through and keeps emitting switch-cases.

The dead-code elimination pass was actually doing us a favor here and
eliminating the excess code later. A flag was added to disable DCE in
order to reveal the problem in a test.

Change-Id: I8ff19fde5e32d0ab73d7c5411da40cb953a446f5
Bug: skia:11352
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372956
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This commit is contained in:
John Stiles 2021-02-20 08:00:43 -05:00
parent 1f95e6c6b4
commit 66c53b9428
7 changed files with 95 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
gn/sksl_tests.gni
View File

@ -479,6 +479,7 @@ sksl_inliner_tests = [
"/sksl/inliner/InlinerWrapsEarlyReturnsWithForLoop.sksl",
"/sksl/inliner/InlinerWrapsSwitchWithReturnInsideWithForLoop.sksl",
"/sksl/inliner/ShortCircuitEvaluationsCannotInlineRightHandSide.sksl",
"/sksl/inliner/StaticSwitch.sksl",
"/sksl/inliner/StructsCanBeInlinedSafely.sksl",
"/sksl/inliner/SwitchWithCastCanBeInlined.sksl",
"/sksl/inliner/SwitchWithoutReturnInsideCanBeInlined.sksl",

19
resources/sksl/inliner/StaticSwitch.sksl Normal file
View File

@ -0,0 +1,19 @@
/*#pragma settings NoDeadCodeElimination*/
uniform half4 colorGreen, colorRed;
float get() {
@switch (2) {
case 1: return abs(1);
case 2: return abs(2); // Only this case should be preserved.
case 3: return abs(3);
case 4: return abs(4);
}
// This won't be removed because dead-code elimination is disabled.
return abs(5);
}
half4 main() {
float result = get();
return result == 2 ? colorGreen : colorRed;
}

54
src/sksl/SkSLCompiler.cpp
View File

@ -732,7 +732,8 @@ void Compiler::simplifyExpression(DefinitionMap& definitions,
case Expression::Kind::kVariableReference: {
const VariableReference& ref = expr->as<VariableReference>();
const Variable* var = ref.variable();
if (ref.refKind() != VariableReference::RefKind::kWrite &&
if (fContext->fConfig->fSettings.fDeadCodeElimination &&
ref.refKind() != VariableReference::RefKind::kWrite &&
ref.refKind() != VariableReference::RefKind::kPointer &&
var->storage() == Variable::Storage::kLocal && !definitions.get(var) &&
optimizationContext->fSilences.find(var) == optimizationContext->fSilences.end()) {
@ -1481,14 +1482,16 @@ bool Compiler::scanCFG(FunctionDefinition& f, ProgramUsage* usage) {
CFG cfg = CFGGenerator().getCFG(f);
this->computeDataFlow(&cfg);
// check for unreachable code
for (size_t i = 0; i < cfg.fBlocks.size(); i++) {
const BasicBlock& block = cfg.fBlocks[i];
if (!block.fIsReachable && !block.fAllowUnreachable && block.fNodes.size()) {
const BasicBlock::Node& node = block.fNodes[0];
int offset = node.isStatement() ? (*node.statement())->fOffset
: (*node.expression())->fOffset;
this->error(offset, String("unreachable"));
if (fContext->fConfig->fSettings.fDeadCodeElimination) {
// Check for unreachable code.
for (size_t i = 0; i < cfg.fBlocks.size(); i++) {
const BasicBlock& block = cfg.fBlocks[i];
if (!block.fIsReachable && !block.fAllowUnreachable && block.fNodes.size()) {
const BasicBlock::Node& node = block.fNodes[0];
int offset = node.isStatement() ? (*node.statement())->fOffset
: (*node.expression())->fOffset;
this->error(offset, String("unreachable"));
}
}
}
if (fErrorCount) {
@ -1518,24 +1521,27 @@ bool Compiler::scanCFG(FunctionDefinition& f, ProgramUsage* usage) {
}
BasicBlock& b = cfg.fBlocks[blockId];
if (blockId > 0 && !b.fIsReachable) {
// Block was reachable before optimization, but has since become unreachable. In
// addition to being dead code, it's broken - since control flow can't reach it, no
// prior variable definitions can reach it, and therefore variables might look to
// have not been properly assigned. Kill it by replacing all statements with Nops.
for (BasicBlock::Node& node : b.fNodes) {
if (node.isStatement() && !(*node.statement())->is<Nop>()) {
// Eliminating a node runs the risk of eliminating that node's exits as
// well. Keep track of this and do a rescan if we are about to access one
// of these.
for (BlockId id : b.fExits) {
eliminatedBlockIds.set(id);
if (fContext->fConfig->fSettings.fDeadCodeElimination) {
if (blockId > 0 && !b.fIsReachable) {
// Block was reachable before optimization, but has since become unreachable. In
// addition to being dead code, it's broken - since control flow can't reach it,
// no prior variable definitions can reach it, and therefore variables might
// look to have not been properly assigned. Kill it by replacing all statements
// with Nops.
for (BasicBlock::Node& node : b.fNodes) {
if (node.isStatement() && !(*node.statement())->is<Nop>()) {
// Eliminating a node runs the risk of eliminating that node's exits as
// well. Keep track of this and do a rescan if we are about to access
// one of these.
for (BlockId id : b.fExits) {
eliminatedBlockIds.set(id);
}
node.setStatement(std::make_unique<Nop>(), usage);
madeChanges = true;
}
node.setStatement(std::make_unique<Nop>(), usage);
madeChanges = true;
}
continue;
}
continue;
}
DefinitionMap definitions = b.fBefore;

3
src/sksl/SkSLMain.cpp
View File

@ -204,6 +204,9 @@ static bool detect_shader_settings(const SkSL::String& text,
static auto s_version450CoreCaps = Factory::Version450Core();
*caps = s_version450CoreCaps.get();
}
if (settingsText.consumeSuffix(" NoDeadCodeElimination")) {
settings->fDeadCodeElimination = false;
}
if (settingsText.consumeSuffix(" FlipY")) {
settings->fFlipY = true;
}

17
src/sksl/SkSLProgramSettings.h
View File

@ -51,14 +51,17 @@ struct ProgramSettings {
// At present, zero is always used by our backends.
int fDefaultUniformSet = 0;
int fDefaultUniformBinding = 0;
// If true, remove any uncalled functions other than main(). Note that a function which
// starts out being used may end up being uncalled after optimization.
bool fRemoveDeadFunctions = true;
// Sets an upper limit on the acceptable amount of code growth from inlining.
// A value of zero will disable the inliner entirely.
int fInlineThreshold = SkSL::kDefaultInlineThreshold;
// true to enable optimization passes
// Enables the SkSL optimizer.
bool fOptimize = true;
// (Requires fOptimize = true) Remove any uncalled functions other than main(). Note that a
// function which starts out being used may end up being uncalled after optimization.
bool fRemoveDeadFunctions = true;
// (Requires fOptimize = true) Uses the control-flow graph to detect and eliminate code within
// a function that has become unreachable due to optimization.
bool fDeadCodeElimination = true;
// (Requires fOptimize = true) When greater than zero, enables the inliner. The threshold value
// sets an upper limit on the acceptable amount of code growth from inlining.
int fInlineThreshold = SkSL::kDefaultInlineThreshold;
// If true, implicit conversions to lower precision numeric types are allowed
// (eg, float to half)
bool fAllowNarrowingConversions = false;

4
src/sksl/SkSLSPIRVCodeGenerator.cpp
View File

@ -198,7 +198,7 @@ void SPIRVCodeGenerator::writeOpCode(SpvOp_ opCode, int length, OutputStream& ou
case SpvOpSwitch: // fall through
case SpvOpBranch: // fall through
case SpvOpBranchConditional:
SkASSERT(fCurrentBlock);
SkASSERT(fCurrentBlock || !fContext.fConfig->fSettings.fDeadCodeElimination);
fCurrentBlock = 0;
break;
case SpvOpConstant: // fall through
@ -236,7 +236,7 @@ void SPIRVCodeGenerator::writeOpCode(SpvOp_ opCode, int length, OutputStream& ou
case SpvOpMemberDecorate:
break;
default:
SkASSERT(fCurrentBlock);
SkASSERT(fCurrentBlock || !fContext.fConfig->fSettings.fDeadCodeElimination);
}
this->writeWord((length << 16) | opCode, out);
}

30
tests/sksl/inliner/StaticSwitch.glsl Normal file
View File

@ -0,0 +1,30 @@
out vec4 sk_FragColor;
uniform vec4 colorGreen;
uniform vec4 colorRed;
vec4 main() {
float _0_get;
for (int _1_loop = 0;_1_loop < 1; _1_loop++) {
{
{
_0_get = abs(2.0);
continue;
}
{
_0_get = abs(3.0);
continue;
}
{
_0_get = abs(4.0);
continue;
}
}
{
_0_get = abs(5.0);
continue;
}
}
float result = _0_get;
return result == 2.0 ? colorGreen : colorRed;
}