Support self-assignment elimination in the constant-folder.
Interestingly, this improves our codegen even with the optimizer fully
enabled, as apparently statement chains like:
`x = true; x = x; x = x;`
were getting transformed by constant-propagation into:
`x = true; x = true; x = true;`
making them no longer candidates for self-assignment elimination.
Change-Id: I6d94a809e94b01a00fd92459fcbce898b3cbbb11
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377100
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
diff --git a/src/sksl/SkSLAnalysis.cpp b/src/sksl/SkSLAnalysis.cpp
index 77bd11c..b4a8fa3 100644
--- a/src/sksl/SkSLAnalysis.cpp
+++ b/src/sksl/SkSLAnalysis.cpp
@@ -562,6 +562,44 @@
IsTrivialExpression(*expr.as<IndexExpression>().base()));
}
+/**
+ * Returns true if both expression trees are the same. The left side is expected to be an lvalue.
+ * This only needs to check for trees that can plausibly terminate in a variable, so some basic
+ * candidates like `FloatLiteral` are missing.
+ */
+bool Analysis::IsSelfAssignment(const Expression& left, const Expression& right) {
+ if (left.kind() != right.kind() || left.type() != right.type()) {
+ return false;
+ }
+
+ switch (left.kind()) {
+ case Expression::Kind::kIntLiteral:
+ return left.as<IntLiteral>().value() == right.as<IntLiteral>().value();
+
+ case Expression::Kind::kFieldAccess:
+ return left.as<FieldAccess>().fieldIndex() == right.as<FieldAccess>().fieldIndex() &&
+ IsSelfAssignment(*left.as<FieldAccess>().base(),
+ *right.as<FieldAccess>().base());
+
+ case Expression::Kind::kIndex:
+ return IsSelfAssignment(*left.as<IndexExpression>().index(),
+ *right.as<IndexExpression>().index()) &&
+ IsSelfAssignment(*left.as<IndexExpression>().base(),
+ *right.as<IndexExpression>().base());
+
+ case Expression::Kind::kSwizzle:
+ return left.as<Swizzle>().components() == right.as<Swizzle>().components() &&
+ IsSelfAssignment(*left.as<Swizzle>().base(), *right.as<Swizzle>().base());
+
+ case Expression::Kind::kVariableReference:
+ return left.as<VariableReference>().variable() ==
+ right.as<VariableReference>().variable();
+
+ default:
+ return false;
+ }
+}
+
static const char* invalid_for_ES2(const ForStatement& loop,
Analysis::UnrollableLoopInfo& loopInfo) {
auto getConstant = [&](const std::unique_ptr<Expression>& expr, double* val) {
diff --git a/src/sksl/SkSLAnalysis.h b/src/sksl/SkSLAnalysis.h
index d33f3c8..dbf6da8c 100644
--- a/src/sksl/SkSLAnalysis.h
+++ b/src/sksl/SkSLAnalysis.h
@@ -88,6 +88,11 @@
// - myStruct.myArrayField[7].xyz
static bool IsTrivialExpression(const Expression& expr);
+ // Returns true if both expression trees are the same. The left side is expected to be an
+ // lvalue. This only needs to check for trees that can plausibly terminate in a variable, so
+ // some basic candidates like `FloatLiteral` are missing.
+ static bool IsSelfAssignment(const Expression& left, const Expression& right);
+
// Is 'expr' a constant-expression, as defined by GLSL 1.0, section 5.10? A constant expression
// is one of:
//
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index d10eb18..dec6b67 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -460,48 +460,9 @@
return is_dead(*b.left(), usage);
}
-/**
- * Returns true if both expression trees are the same. The left side is expected to be an lvalue.
- * This only needs to check for trees that can plausibly terminate in a variable, so some basic
- * candidates like `FloatLiteral` are missing.
- */
-static bool is_matching_expression_tree(const Expression& left, const Expression& right) {
- if (left.kind() != right.kind() || left.type() != right.type()) {
- return false;
- }
-
- switch (left.kind()) {
- case Expression::Kind::kIntLiteral:
- return left.as<IntLiteral>().value() == right.as<IntLiteral>().value();
-
- case Expression::Kind::kFieldAccess:
- return left.as<FieldAccess>().fieldIndex() == right.as<FieldAccess>().fieldIndex() &&
- is_matching_expression_tree(*left.as<FieldAccess>().base(),
- *right.as<FieldAccess>().base());
-
- case Expression::Kind::kIndex:
- return is_matching_expression_tree(*left.as<IndexExpression>().index(),
- *right.as<IndexExpression>().index()) &&
- is_matching_expression_tree(*left.as<IndexExpression>().base(),
- *right.as<IndexExpression>().base());
-
- case Expression::Kind::kSwizzle:
- return left.as<Swizzle>().components() == right.as<Swizzle>().components() &&
- is_matching_expression_tree(*left.as<Swizzle>().base(),
- *right.as<Swizzle>().base());
-
- case Expression::Kind::kVariableReference:
- return left.as<VariableReference>().variable() ==
- right.as<VariableReference>().variable();
-
- default:
- return false;
- }
-}
-
static bool self_assignment(const BinaryExpression& b) {
return b.getOperator().kind() == Token::Kind::TK_EQ &&
- is_matching_expression_tree(*b.left(), *b.right());
+ Analysis::IsSelfAssignment(*b.left(), *b.right());
}
void Compiler::computeDataFlow(CFG* cfg) {
diff --git a/src/sksl/SkSLConstantFolder.cpp b/src/sksl/SkSLConstantFolder.cpp
index fc86d90..355b53c 100644
--- a/src/sksl/SkSLConstantFolder.cpp
+++ b/src/sksl/SkSLConstantFolder.cpp
@@ -195,6 +195,13 @@
return right.clone();
}
+ // If this is the assignment operator, and both sides are the same trivial expression, this is
+ // self-assignment (i.e., `var = var`) and can be reduced to just a variable reference (`var`).
+ // This can happen when other parts of the assignment are optimized away.
+ if (op.kind() == Token::Kind::TK_EQ && Analysis::IsSelfAssignment(left, right)) {
+ return right.clone();
+ }
+
// Simplify the expression when both sides are constant Boolean literals.
if (left.is<BoolLiteral>() && right.is<BoolLiteral>()) {
bool leftVal = left.as<BoolLiteral>().value();