Convert IRGenerator::convertIf to IfStatement::Make.
This lets the Rehydrator and Inliner benefit from static optimization
opportunities, like converting `if (false) {...}` to a Nop.
Change-Id: I70b4fceab84c50ea270dc894b0d6fe0e7e2369eb
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375777
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
diff --git a/gn/sksl.gni b/gn/sksl.gni
index d0c23df..4e94736 100644
--- a/gn/sksl.gni
+++ b/gn/sksl.gni
@@ -107,6 +107,7 @@
"$_src/sksl/ir/SkSLFunctionPrototype.h",
"$_src/sksl/ir/SkSLFunctionReference.h",
"$_src/sksl/ir/SkSLIRNode.h",
+ "$_src/sksl/ir/SkSLIfStatement.cpp",
"$_src/sksl/ir/SkSLIfStatement.h",
"$_src/sksl/ir/SkSLIndexExpression.h",
"$_src/sksl/ir/SkSLInlineMarker.h",
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 4c8102e..353a186 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -561,45 +561,22 @@
if (!ifTrue) {
return nullptr;
}
+ if (this->detectVarDeclarationWithoutScope(*ifTrue)) {
+ return nullptr;
+ }
std::unique_ptr<Statement> ifFalse;
if (iter != n.end()) {
ifFalse = this->convertStatement(*(iter++));
if (!ifFalse) {
return nullptr;
}
+ if (this->detectVarDeclarationWithoutScope(*ifFalse)) {
+ return nullptr;
+ }
}
bool isStatic = n.getBool();
- return this->convertIf(n.fOffset, isStatic, std::move(test), std::move(ifTrue),
- std::move(ifFalse));
-}
-
-std::unique_ptr<Statement> IRGenerator::convertIf(int offset, bool isStatic,
- std::unique_ptr<Expression> test,
- std::unique_ptr<Statement> ifTrue,
- std::unique_ptr<Statement> ifFalse) {
- test = this->coerce(std::move(test), *fContext.fTypes.fBool);
- if (!test) {
- return nullptr;
- }
- if (this->detectVarDeclarationWithoutScope(*ifTrue)) {
- return nullptr;
- }
- if (ifFalse && this->detectVarDeclarationWithoutScope(*ifFalse)) {
- return nullptr;
- }
- if (test->is<BoolLiteral>()) {
- // Static Boolean values can fold down to a single branch.
- if (test->as<BoolLiteral>().value()) {
- return ifTrue;
- }
- if (ifFalse) {
- return ifFalse;
- }
- // False, but no else-clause. Not an error, so don't return null!
- return std::make_unique<Nop>();
- }
- return std::make_unique<IfStatement>(offset, isStatic, std::move(test), std::move(ifTrue),
- std::move(ifFalse));
+ return IfStatement::Make(fContext, n.fOffset, isStatic, std::move(test),
+ std::move(ifTrue), std::move(ifFalse));
}
std::unique_ptr<Statement> IRGenerator::convertFor(int offset,
diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h
index 8a59453..7e98bb9 100644
--- a/src/sksl/SkSLIRGenerator.h
+++ b/src/sksl/SkSLIRGenerator.h
@@ -212,10 +212,6 @@
std::unique_ptr<Expression> convertIdentifier(int offset, StringFragment identifier);
std::unique_ptr<Expression> convertIdentifier(const ASTNode& identifier);
std::unique_ptr<Statement> convertIf(const ASTNode& s);
- std::unique_ptr<Statement> convertIf(int offset, bool isStatic,
- std::unique_ptr<Expression> test,
- std::unique_ptr<Statement> ifTrue,
- std::unique_ptr<Statement> ifFalse);
std::unique_ptr<InterfaceBlock> convertInterfaceBlock(const ASTNode& s);
Modifiers convertModifiers(const Modifiers& m);
std::unique_ptr<Expression> convertPrefixExpression(const ASTNode& expression);
diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
index e8ac206..59e6966 100644
--- a/src/sksl/SkSLInliner.cpp
+++ b/src/sksl/SkSLInliner.cpp
@@ -467,8 +467,8 @@
}
case Statement::Kind::kIf: {
const IfStatement& i = statement.as<IfStatement>();
- return std::make_unique<IfStatement>(offset, i.isStatic(), expr(i.test()),
- stmt(i.ifTrue()), stmt(i.ifFalse()));
+ return IfStatement::Make(*fContext, offset, i.isStatic(), expr(i.test()),
+ stmt(i.ifTrue()), stmt(i.ifFalse()));
}
case Statement::Kind::kInlineMarker:
case Statement::Kind::kNop:
diff --git a/src/sksl/SkSLRehydrator.cpp b/src/sksl/SkSLRehydrator.cpp
index e935d48..a220235 100644
--- a/src/sksl/SkSLRehydrator.cpp
+++ b/src/sksl/SkSLRehydrator.cpp
@@ -386,9 +386,8 @@
std::unique_ptr<Expression> test = this->expression();
std::unique_ptr<Statement> ifTrue = this->statement();
std::unique_ptr<Statement> ifFalse = this->statement();
- return std::unique_ptr<Statement>(new IfStatement(-1, isStatic, std::move(test),
- std::move(ifTrue),
- std::move(ifFalse)));
+ return IfStatement::Make(fContext, /*offset=*/-1, isStatic, std::move(test),
+ std::move(ifTrue), std::move(ifFalse));
}
case Rehydrator::kInlineMarker_Command: {
const FunctionDeclaration* funcDecl = this->symbolRef<FunctionDeclaration>(
diff --git a/src/sksl/dsl/DSLCore.cpp b/src/sksl/dsl/DSLCore.cpp
index ebe5719..1200be1 100644
--- a/src/sksl/dsl/DSLCore.cpp
+++ b/src/sksl/dsl/DSLCore.cpp
@@ -98,8 +98,8 @@
}
static DSLStatement If(DSLExpression test, DSLStatement ifTrue, DSLStatement ifFalse) {
- return DSLWriter::IRGenerator().convertIf(/*offset=*/-1, /*isStatic=*/false, test.release(),
- ifTrue.release(), ifFalse.release());
+ return IfStatement::Make(DSLWriter::Context(), /*offset=*/-1, /*isStatic=*/false,
+ test.release(), ifTrue.release(), ifFalse.release());
}
static DSLStatement Return(DSLExpression value) {
diff --git a/src/sksl/ir/SkSLIfStatement.cpp b/src/sksl/ir/SkSLIfStatement.cpp
new file mode 100644
index 0000000..2b7237f
--- /dev/null
+++ b/src/sksl/ir/SkSLIfStatement.cpp
@@ -0,0 +1,57 @@
+/*
+ * Copyright 2021 Google LLC
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "src/sksl/SkSLContext.h"
+#include "src/sksl/ir/SkSLBoolLiteral.h"
+#include "src/sksl/ir/SkSLIfStatement.h"
+#include "src/sksl/ir/SkSLNop.h"
+#include "src/sksl/ir/SkSLType.h"
+
+namespace SkSL {
+
+std::unique_ptr<Statement> IfStatement::clone() const {
+ return std::make_unique<IfStatement>(fOffset, this->isStatic(), this->test()->clone(),
+ this->ifTrue()->clone(),
+ this->ifFalse() ? this->ifFalse()->clone() : nullptr);
+}
+
+String IfStatement::description() const {
+ String result;
+ if (this->isStatic()) {
+ result += "@";
+ }
+ result += "if (" + this->test()->description() + ") " + this->ifTrue()->description();
+ if (this->ifFalse()) {
+ result += " else " + this->ifFalse()->description();
+ }
+ return result;
+}
+
+std::unique_ptr<Statement> IfStatement::Make(const Context& context, int offset, bool isStatic,
+ std::unique_ptr<Expression> test,
+ std::unique_ptr<Statement> ifTrue,
+ std::unique_ptr<Statement> ifFalse) {
+ test = context.fTypes.fBool->coerceExpression(std::move(test), context);
+ if (!test) {
+ return nullptr;
+ }
+ if (test->is<BoolLiteral>()) {
+ // Static Boolean values can fold down to a single branch.
+ if (test->as<BoolLiteral>().value()) {
+ return ifTrue;
+ }
+ if (ifFalse) {
+ return ifFalse;
+ }
+ // False, but no else-clause. Not an error, so don't return null!
+ return std::make_unique<Nop>();
+ }
+ return std::make_unique<IfStatement>(offset, isStatic, std::move(test), std::move(ifTrue),
+ std::move(ifFalse));
+}
+
+} // namespace SkSL
diff --git a/src/sksl/ir/SkSLIfStatement.h b/src/sksl/ir/SkSLIfStatement.h
index 56aaab9..07df587 100644
--- a/src/sksl/ir/SkSLIfStatement.h
+++ b/src/sksl/ir/SkSLIfStatement.h
@@ -11,6 +11,8 @@
#include "src/sksl/ir/SkSLExpression.h"
#include "src/sksl/ir/SkSLStatement.h"
+#include <memory>
+
namespace SkSL {
/**
@@ -20,6 +22,8 @@
public:
static constexpr Kind kStatementKind = Kind::kIf;
+ // Use IfStatement::Make to automatically flatten out `if` statements that can be resolved at
+ // IR generation time.
IfStatement(int offset, bool isStatic, std::unique_ptr<Expression> test,
std::unique_ptr<Statement> ifTrue, std::unique_ptr<Statement> ifFalse)
: INHERITED(offset, kStatementKind)
@@ -28,6 +32,11 @@
, fIfFalse(std::move(ifFalse))
, fIsStatic(isStatic) {}
+ static std::unique_ptr<Statement> Make(const Context& context, int offset, bool isStatic,
+ std::unique_ptr<Expression> test,
+ std::unique_ptr<Statement> ifTrue,
+ std::unique_ptr<Statement> ifFalse);
+
bool isStatic() const {
return fIsStatic;
}
@@ -56,25 +65,9 @@
return fIfFalse;
}
- std::unique_ptr<Statement> clone() const override {
- return std::unique_ptr<Statement>(new IfStatement(fOffset, this->isStatic(),
- this->test()->clone(),
- this->ifTrue()->clone(),
- this->ifFalse() ? this->ifFalse()->clone()
- : nullptr));
- }
+ std::unique_ptr<Statement> clone() const override;
- String description() const override {
- String result;
- if (this->isStatic()) {
- result += "@";
- }
- result += "if (" + this->test()->description() + ") " + this->ifTrue()->description();
- if (this->ifFalse()) {
- result += " else " + this->ifFalse()->description();
- }
- return result;
- }
+ String description() const override;
private:
std::unique_ptr<Expression> fTest;
