Move static-switch and static-if tests out of scanCFG.

We don't need to do these tests every time we run CFG optimization; we
can do them once at the end of optimization, as a separate step.

Change-Id: If0e72fbacb938b62387fd2ffdbf34d1153bf3bd4
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369481
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
John Stiles 2021-02-12 09:17:53 -05:00 committed by Skia Commit-Bot
parent 1526444aa4
commit bb1505f2ab
4 changed files with 61 additions and 41 deletions

View File

@ -1,10 +1,9 @@
void non_constant_test_in_static_switch() {
half4 main() {
int x = int(sqrt(1));
@switch (x) {
case 1:
sk_FragColor = half4(1);
break;
return half4(1);
default:
sk_FragColor = half4(0);
return half4(0);
}
}

View File

@ -1171,14 +1171,14 @@ static bool contains_conditional_break(Statement& stmt) {
bool visitStatement(const Statement& stmt) override {
switch (stmt.kind()) {
case Statement::Kind::kBlock:
return this->INHERITED::visitStatement(stmt);
return INHERITED::visitStatement(stmt);
case Statement::Kind::kBreak:
return fInConditional > 0;
case Statement::Kind::kIf: {
++fInConditional;
bool result = this->INHERITED::visitStatement(stmt);
bool result = INHERITED::visitStatement(stmt);
--fInConditional;
return result;
}
@ -1203,7 +1203,7 @@ static bool contains_unconditional_break(Statement& stmt) {
bool visitStatement(const Statement& stmt) override {
switch (stmt.kind()) {
case Statement::Kind::kBlock:
return this->INHERITED::visitStatement(stmt);
return INHERITED::visitStatement(stmt);
case Statement::Kind::kBreak:
return true;
@ -1552,39 +1552,6 @@ bool Compiler::scanCFG(FunctionDefinition& f, ProgramUsage* usage) {
} while (optimizationContext.fUpdated);
SkASSERT(!optimizationContext.fNeedsRescan);
// verify static ifs & switches, clean up dead variable decls
for (BasicBlock& b : cfg.fBlocks) {
for (auto iter = b.fNodes.begin(); iter != b.fNodes.end() &&
!optimizationContext.fNeedsRescan;) {
if (iter->isStatement()) {
const Statement& s = **iter->statement();
switch (s.kind()) {
case Statement::Kind::kIf:
if (s.as<IfStatement>().isStatic() &&
!fIRGenerator->fSettings->fPermitInvalidStaticTests) {
this->error(s.fOffset, "static if has non-static test");
}
++iter;
break;
case Statement::Kind::kSwitch:
if (s.as<SwitchStatement>().isStatic() &&
!fIRGenerator->fSettings->fPermitInvalidStaticTests &&
optimizationContext.fSilences.find(&s) ==
optimizationContext.fSilences.end()) {
this->error(s.fOffset, "static switch has non-static test");
}
++iter;
break;
default:
++iter;
break;
}
} else {
++iter;
}
}
}
// check for missing return
if (f.declaration().returnType() != *fContext->fTypes.fVoid) {
if (cfg.fBlocks[cfg.fExit].fIsReachable) {
@ -1652,6 +1619,52 @@ std::unique_ptr<Program> Compiler::convertProgram(
return success ? std::move(program) : nullptr;
}
void Compiler::verifyStaticTests(const Program& program) {
class StaticTestVerifier : public ProgramVisitor {
public:
StaticTestVerifier(ErrorReporter* r) : fReporter(r) {}
using ProgramVisitor::visitProgramElement;
bool visitStatement(const Statement& stmt) override {
switch (stmt.kind()) {
case Statement::Kind::kIf:
if (stmt.as<IfStatement>().isStatic()) {
fReporter->error(stmt.fOffset, "static if has non-static test");
}
break;
case Statement::Kind::kSwitch:
if (stmt.as<SwitchStatement>().isStatic()) {
fReporter->error(stmt.fOffset, "static switch has non-static test");
}
break;
default:
break;
}
return INHERITED::visitStatement(stmt);
}
private:
using INHERITED = ProgramVisitor;
ErrorReporter* fReporter;
};
// If invalid static tests are permitted, we don't need to check anything.
if (fIRGenerator->fSettings->fPermitInvalidStaticTests) {
return;
}
// Check all of the program's owned elements. (Built-in elements are assumed to be valid.)
StaticTestVerifier visitor{this};
for (const std::unique_ptr<ProgramElement>& element : program.ownedElements()) {
if (element->is<FunctionDefinition>()) {
visitor.visitProgramElement(*element);
}
}
}
bool Compiler::optimize(LoadedModule& module) {
SkASSERT(!fErrorCount);
Program::Settings settings;
@ -1757,6 +1770,11 @@ bool Compiler::optimize(Program& program) {
break;
}
}
if (fErrorCount == 0) {
this->verifyStaticTests(program);
}
return fErrorCount == 0;
}

View File

@ -181,6 +181,9 @@ private:
void scanCFG(CFG* cfg, BlockId block, SkBitSet* processedSet);
void computeDataFlow(CFG* cfg);
/** Verifies that @if and @switch statements were actually optimized away. */
void verifyStaticTests(const Program& program);
/**
* Simplifies the expression pointed to by iter (in both the IR and CFG structures), if
* possible.

View File

@ -296,7 +296,7 @@ struct Program {
// Can be used to iterate over *just* the elements owned by the Program, not shared builtins.
// The iterator's value type is 'std::unique_ptr<ProgramElement>', and mutation is allowed.
const std::vector<std::unique_ptr<ProgramElement>>& ownedElements() { return fElements; }
const std::vector<std::unique_ptr<ProgramElement>>& ownedElements() const { return fElements; }
Kind fKind;
std::unique_ptr<String> fSource;