Revert "Fix incorrect 'unreachable code' error in SkSL"

This reverts commit 67d2d73d06.

Reason for revert: Triggers errors in some shaders.

Original change's description:
> Fix incorrect 'unreachable code' error in SkSL
>
> This trades one error for another (a potential for incorrect use of
> unassigned variables). False-positives for unassigned variables are
> straightforward to workaround (and produce code that still looks
> reasonable). Working around unreachable code errors is tricky, and
> likely to produce non-idiomatic code. This change also makes the data
> flow analysis of all loop constructs more similar - for loops were
> behaving very differently from while loops.
>
> Note that this effectively a revert of:
>   https://skia-review.googlesource.com/c/skia/+/18121/
>
> Change-Id: Ib85d90b22cac8addfb106459c0a5f5616a89c3eb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344957
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>

TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com

Change-Id: I84b93cc7f9309446dcc0e949e90908df31a1ff9c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345119
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Osman 2020-12-16 18:21:05 +00:00 committed by Skia Commit-Bot
parent 04cf690185
commit 21a1afe7f8
3 changed files with 10 additions and 4 deletions

View File

@ -602,8 +602,13 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) {
fLoopExits.push(loopExit); fLoopExits.push(loopExit);
if (f.test()) { if (f.test()) {
this->addExpression(cfg, &f.test(), /*constantPropagate=*/true); this->addExpression(cfg, &f.test(), /*constantPropagate=*/true);
BlockId test = cfg.fCurrent; // this isn't quite right; we should have an exit from here to the loop exit, and
cfg.addExit(test, loopExit); // remove the exit from the loop body to the loop exit. Structuring it like this
// forces the optimizer to believe that the loop body is always executed at least
// once. While not strictly correct, this avoids incorrect "variable not assigned"
// errors on variables which are assigned within the loop. The correct solution to
// this is to analyze the loop to see whether or not at least one iteration is
// guaranteed to happen, but for the time being we take the easy way out.
} }
cfg.newBlock(); cfg.newBlock();
this->addStatement(cfg, &f.statement()); this->addStatement(cfg, &f.statement());
@ -613,6 +618,7 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) {
this->addExpression(cfg, &f.next(), /*constantPropagate=*/true); this->addExpression(cfg, &f.next(), /*constantPropagate=*/true);
} }
cfg.addExit(cfg.fCurrent, loopStart); cfg.addExit(cfg.fCurrent, loopStart);
cfg.addExit(cfg.fCurrent, loopExit);
fLoopContinues.pop(); fLoopContinues.pop();
fLoopExits.pop(); fLoopExits.pop();
cfg.fCurrent = loopExit; cfg.fCurrent = loopExit;

View File

@ -3,4 +3,5 @@ void call_after_return() { return; call_after_return(); }
void code_after_continue() { for (;;) { continue; int x = 1; } } void code_after_continue() { for (;;) { continue; int x = 1; } }
void code_after_if_else_both_sided_exit() { if (true) return; else discard; return; } void code_after_if_else_both_sided_exit() { if (true) return; else discard; return; }
// Not detected today
void code_after_infinite_loop() { for (;;) {} return; } void code_after_infinite_loop() { for (;;) {} return; }

View File

@ -4,5 +4,4 @@ error: 1: unreachable
error: 2: unreachable error: 2: unreachable
error: 3: unreachable error: 3: unreachable
error: 4: unreachable error: 4: unreachable
error: 6: unreachable 4 errors
5 errors