Convert make_unique<ExpressionStatement> to ExpressionStatement::Make.
This allows for elimination expression-statements with no side effects
when they are first created, rather than later on in the control-flow
optimization pass.
Change-Id: I09ec8421a3af7fa37f98b0405657198c0a24f2ac
Bug: skia:11342, skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376136
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
diff --git a/gn/sksl.gni b/gn/sksl.gni
index 75766eb..37fb4c1 100644
--- a/gn/sksl.gni
+++ b/gn/sksl.gni
@@ -93,6 +93,7 @@
"$_src/sksl/ir/SkSLDoStatement.h",
"$_src/sksl/ir/SkSLEnum.h",
"$_src/sksl/ir/SkSLExpression.h",
+ "$_src/sksl/ir/SkSLExpressionStatement.cpp",
"$_src/sksl/ir/SkSLExpressionStatement.h",
"$_src/sksl/ir/SkSLExtension.h",
"$_src/sksl/ir/SkSLExternalFunctionCall.h",
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 5319c36..d0ecf78 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -707,7 +707,7 @@
if (!e) {
return nullptr;
}
- return std::unique_ptr<Statement>(new ExpressionStatement(std::move(e)));
+ return ExpressionStatement::Make(fContext, std::move(e));
}
std::unique_ptr<Statement> IRGenerator::convertReturn(int offset,
@@ -785,12 +785,12 @@
StatementArray loopBody;
loopBody.reserve_back(2);
- loopBody.push_back(std::make_unique<ExpressionStatement>(this->call(/*offset=*/-1,
- *invokeDecl,
- ExpressionArray{})));
- loopBody.push_back(std::make_unique<ExpressionStatement>(this->call(/*offset=*/-1,
- std::move(endPrimitive),
- ExpressionArray{})));
+ loopBody.push_back(ExpressionStatement::Make(fContext, this->call(/*offset=*/-1,
+ *invokeDecl,
+ ExpressionArray{})));
+ loopBody.push_back(ExpressionStatement::Make(fContext, this->call(/*offset=*/-1,
+ std::move(endPrimitive),
+ ExpressionArray{})));
auto assignment = std::make_unique<BinaryExpression>(
/*offset=*/-1,
std::make_unique<VariableReference>(/*offset=*/-1, loopIdx,
@@ -798,7 +798,7 @@
Token::Kind::TK_EQ,
std::make_unique<IntLiteral>(fContext, /*offset=*/-1, /*value=*/0),
fContext.fTypes.fInt.get());
- auto initializer = std::make_unique<ExpressionStatement>(std::move(assignment));
+ auto initializer = ExpressionStatement::Make(fContext, std::move(assignment));
auto loop = ForStatement::Make(
fContext, /*offset=*/-1, std::move(initializer), std::move(test), std::move(next),
std::make_unique<Block>(/*offset=*/-1, std::move(loopBody)), fSymbolTable);
@@ -865,7 +865,7 @@
std::unique_ptr<Expression> result = Op(Pos(), Token::Kind::TK_EQ,
Constructor::Make(fContext, /*offset=*/-1,
*fContext.fTypes.fFloat4, std::move(children)));
- return std::make_unique<ExpressionStatement>(std::move(result));
+ return ExpressionStatement::Make(fContext, std::move(result));
}
void IRGenerator::checkModifiers(int offset, const Modifiers& modifiers, int permitted) {
diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
index 1ec491b..098185f 100644
--- a/src/sksl/SkSLInliner.cpp
+++ b/src/sksl/SkSLInliner.cpp
@@ -454,7 +454,7 @@
}
case Statement::Kind::kExpression: {
const ExpressionStatement& e = statement.as<ExpressionStatement>();
- return std::make_unique<ExpressionStatement>(expr(e.expression()));
+ return ExpressionStatement::Make(*fContext, expr(e.expression()));
}
case Statement::Kind::kFor: {
const ForStatement& f = statement.as<ForStatement>();
@@ -499,8 +499,9 @@
// For more complex functions, assign their result into a variable.
SkASSERT(*resultExpr);
- auto assignment =
- std::make_unique<ExpressionStatement>(std::make_unique<BinaryExpression>(
+ auto assignment = ExpressionStatement::Make(
+ *fContext,
+ std::make_unique<BinaryExpression>(
offset,
clone_with_ref_kind(**resultExpr, VariableReference::RefKind::kWrite),
Token::Kind::TK_EQ,
@@ -519,8 +520,7 @@
}
// Functions without early returns aren't wrapped in a for loop and don't need to worry
// about breaking out of the control flow.
- return std::move(assignment);
-
+ return assignment;
}
case Statement::Kind::kSwitch: {
const SwitchStatement& ss = statement.as<SwitchStatement>();
@@ -748,8 +748,9 @@
for (int i : argsToCopyBack) {
const Variable* p = function.declaration().parameters()[i];
SkASSERT(varMap.find(p) != varMap.end());
- inlineStatements->push_back(
- std::make_unique<ExpressionStatement>(std::make_unique<BinaryExpression>(
+ inlineStatements->push_back(ExpressionStatement::Make(
+ *fContext,
+ std::make_unique<BinaryExpression>(
offset,
clone_with_ref_kind(*arguments[i], VariableReference::RefKind::kWrite),
Token::Kind::TK_EQ,
diff --git a/src/sksl/SkSLRehydrator.cpp b/src/sksl/SkSLRehydrator.cpp
index a220235..781827c 100644
--- a/src/sksl/SkSLRehydrator.cpp
+++ b/src/sksl/SkSLRehydrator.cpp
@@ -368,7 +368,7 @@
}
case Rehydrator::kExpressionStatement_Command: {
std::unique_ptr<Expression> expr = this->expression();
- return std::unique_ptr<Statement>(new ExpressionStatement(std::move(expr)));
+ return ExpressionStatement::Make(fContext, std::move(expr));
}
case Rehydrator::kFor_Command: {
std::unique_ptr<Statement> initializer = this->statement();
diff --git a/src/sksl/ir/SkSLExpressionStatement.cpp b/src/sksl/ir/SkSLExpressionStatement.cpp
new file mode 100644
index 0000000..17f5de1
--- /dev/null
+++ b/src/sksl/ir/SkSLExpressionStatement.cpp
@@ -0,0 +1,27 @@
+/*
+ * 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/SkSLProgramSettings.h"
+#include "src/sksl/ir/SkSLExpressionStatement.h"
+#include "src/sksl/ir/SkSLNop.h"
+
+namespace SkSL {
+
+std::unique_ptr<Statement> ExpressionStatement::Make(const Context& context,
+ std::unique_ptr<Expression> expr) {
+ if (context.fConfig->fSettings.fOptimize) {
+ // Expression-statements without any side effect can be replaced with a Nop.
+ if (!expr->hasSideEffects()) {
+ return std::make_unique<Nop>();
+ }
+ }
+
+ return std::make_unique<ExpressionStatement>(std::move(expr));
+}
+
+} // namespace SkSL
diff --git a/src/sksl/ir/SkSLExpressionStatement.h b/src/sksl/ir/SkSLExpressionStatement.h
index b14964f..3b99492 100644
--- a/src/sksl/ir/SkSLExpressionStatement.h
+++ b/src/sksl/ir/SkSLExpressionStatement.h
@@ -24,6 +24,9 @@
: INHERITED(expression->fOffset, kStatementKind)
, fExpression(std::move(expression)) {}
+ static std::unique_ptr<Statement> Make(const Context& context,
+ std::unique_ptr<Expression> expr);
+
const std::unique_ptr<Expression>& expression() const {
return fExpression;
}
@@ -33,7 +36,7 @@
}
std::unique_ptr<Statement> clone() const override {
- return std::unique_ptr<Statement>(new ExpressionStatement(this->expression()->clone()));
+ return std::make_unique<ExpressionStatement>(this->expression()->clone());
}
String description() const override {
