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>
This commit is contained in:
John Stiles 2021-03-01 17:02:28 -05:00 committed by Skia Commit-Bot
parent e2aec43cfc
commit 95d0badecf
6 changed files with 51 additions and 97 deletions

View File

@ -562,6 +562,44 @@ bool Analysis::IsTrivialExpression(const Expression& expr) {
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) {

View File

@ -88,6 +88,11 @@ struct Analysis {
// - 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:
//

View File

@ -460,48 +460,9 @@ static bool dead_assignment(const BinaryExpression& b, ProgramUsage* usage) {
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) {

View File

@ -195,6 +195,13 @@ std::unique_ptr<Expression> ConstantFolder::Simplify(const Context& context,
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();

View File

@ -1,4 +1 @@
### Compilation failed:
error: 1: 'i' has not been assigned
1 error

View File

@ -6,33 +6,6 @@ uniform vec4 colorGreen;
bool test_int() {
int unknown = int(unknownInput);
bool ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = true;
ok = ivec4(unknown) == ivec4(unknown);
ok = ok && ivec4(unknown) == ivec4(unknown);
ok = ok && ivec4(unknown) == ivec4(unknown);
@ -54,33 +27,6 @@ bool test_int() {
vec4 main() {
float _0_unknown = unknownInput;
bool _1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = true;
_1_ok = vec4(_0_unknown) == vec4(_0_unknown);
_1_ok = _1_ok && vec4(_0_unknown) == vec4(_0_unknown);
_1_ok = _1_ok && vec4(_0_unknown) == vec4(_0_unknown);