Skip to content

Commit 74ebd7e

Browse files
johnstiles-googleSkia Commit-Bot
authored andcommitted
Add support for inlining switches with returns inside.
Because we use `continue` for flow control handling now, we can escape from the middle of a switch statement. This wasn't possible when we used `break`. This unlocks some pretty stellar optimization opportunities if the switch value can be determined at compile time; see BlendEnum for an example. Change-Id: Id29be92c343c10fd604683a80c5d5bd2bd070cb0 Bug: skia:11097 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345419 Commit-Queue: John Stiles <[email protected]> Auto-Submit: John Stiles <[email protected]> Reviewed-by: Ethan Nicholas <[email protected]>
1 parent e0049aa commit 74ebd7e

10 files changed

+104
-2688
lines changed

gn/sksl_tests.gni

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,9 +398,10 @@ sksl_inliner_tests = [
398398
"$_tests/sksl/inliner/InlineWithUnnecessaryBlocks.sksl",
399399
"$_tests/sksl/inliner/InlinerAvoidsVariableNameOverlap.sksl",
400400
"$_tests/sksl/inliner/InlinerManglesNames.sksl",
401+
"$_tests/sksl/inliner/InlinerWrapsEarlyReturnsWithForLoop.sksl",
402+
"$_tests/sksl/inliner/InlinerWrapsSwitchWithReturnInsideWithForLoop.sksl",
401403
"$_tests/sksl/inliner/ShortCircuitEvaluationsCannotInlineRightHandSide.sksl",
402404
"$_tests/sksl/inliner/SwitchWithCastCanBeInlined.sksl",
403-
"$_tests/sksl/inliner/SwitchWithReturnInsideCannotBeInlined.sksl",
404405
"$_tests/sksl/inliner/SwitchWithoutReturnInsideCanBeInlined.sksl",
405406
"$_tests/sksl/inliner/SwizzleCanBeInlinedDirectly.sksl",
406407
"$_tests/sksl/inliner/TernaryResultsCannotBeInlined.sksl",
@@ -447,7 +448,6 @@ sksl_settings_tests = [
447448
"$_tests/sksl/glsl/TypePrecision.sksl",
448449
"$_tests/sksl/inliner/ExponentialGrowth.sksl",
449450
"$_tests/sksl/inliner/InlinerCanBeDisabled.sksl",
450-
"$_tests/sksl/inliner/InlinerWrapsEarlyReturnsWithForLoop.sksl",
451451
"$_tests/sksl/shared/Derivatives.sksl",
452452
"$_tests/sksl/shared/DerivativesFlipY.sksl",
453453
"$_tests/sksl/workarounds/AbsInt.sksl",

src/sksl/SkSLInliner.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -94,26 +94,25 @@ static int count_returns_at_end_of_control_flow(const FunctionDefinition& funcDe
9494
return CountReturnsAtEndOfControlFlow{funcDef}.fNumReturns;
9595
}
9696

97-
static int count_returns_in_breakable_constructs(const FunctionDefinition& funcDef) {
98-
class CountReturnsInBreakableConstructs : public ProgramVisitor {
97+
static int count_returns_in_continuable_constructs(const FunctionDefinition& funcDef) {
98+
class CountReturnsInContinuableConstructs : public ProgramVisitor {
9999
public:
100-
CountReturnsInBreakableConstructs(const FunctionDefinition& funcDef) {
100+
CountReturnsInContinuableConstructs(const FunctionDefinition& funcDef) {
101101
this->visitProgramElement(funcDef);
102102
}
103103

104104
bool visitStatement(const Statement& stmt) override {
105105
switch (stmt.kind()) {
106-
case Statement::Kind::kSwitch:
107106
case Statement::Kind::kDo:
108107
case Statement::Kind::kFor: {
109-
++fInsideBreakableConstruct;
108+
++fInsideContinuableConstruct;
110109
bool result = INHERITED::visitStatement(stmt);
111-
--fInsideBreakableConstruct;
110+
--fInsideContinuableConstruct;
112111
return result;
113112
}
114113

115114
case Statement::Kind::kReturn:
116-
fNumReturns += (fInsideBreakableConstruct > 0) ? 1 : 0;
115+
fNumReturns += (fInsideContinuableConstruct > 0) ? 1 : 0;
117116
[[fallthrough]];
118117

119118
default:
@@ -122,11 +121,11 @@ static int count_returns_in_breakable_constructs(const FunctionDefinition& funcD
122121
}
123122

124123
int fNumReturns = 0;
125-
int fInsideBreakableConstruct = 0;
124+
int fInsideContinuableConstruct = 0;
126125
using INHERITED = ProgramVisitor;
127126
};
128127

129-
return CountReturnsInBreakableConstructs{funcDef}.fNumReturns;
128+
return CountReturnsInContinuableConstructs{funcDef}.fNumReturns;
130129
}
131130

132131
static bool contains_recursive_call(const FunctionDeclaration& funcDecl) {
@@ -834,10 +833,11 @@ bool Inliner::isSafeToInline(const FunctionDefinition* functionDef) {
834833
return false;
835834
}
836835

837-
// We don't have any mechanism to simulate early returns within a breakable construct
838-
// (switch/for/do/while), so we can't inline if there's a return inside one.
839-
bool hasReturnInBreakableConstruct = (count_returns_in_breakable_constructs(*functionDef) > 0);
840-
return !hasReturnInBreakableConstruct;
836+
// We don't have any mechanism to simulate early returns within a construct that supports
837+
// continues (for/do/while), so we can't inline if there's a return inside one.
838+
bool hasReturnInContinuableConstruct =
839+
(count_returns_in_continuable_constructs(*functionDef) > 0);
840+
return !hasReturnInContinuableConstruct;
841841
}
842842

843843
// A candidate function for inlining, containing everything that `inlineCall` needs.

0 commit comments

Comments
 (0)