Skip to content

Commit 7096a3f

Browse files
authored
Simplify boolean expressions in Java AST (#10279)
Split from #10261 (see comments in that PR for discussion). Fixes #10278
1 parent 34fd51e commit 7096a3f

2 files changed

Lines changed: 36 additions & 3 deletions

File tree

dev/core/src/com/google/gwt/dev/jjs/impl/Simplifier.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -429,8 +429,13 @@ public static JExpression simplifyAnd(JBinaryOperation expression) {
429429
if (rhs instanceof JBooleanLiteral) {
430430
if (((JBooleanLiteral) rhs).getValue()) {
431431
return lhs;
432-
} else if (!lhs.hasSideEffects()) {
433-
// Do not remove lhs if it had side effects
432+
} else if (lhs.hasSideEffects()) {
433+
// if side effect, rewrite a() && false to (a(), false)
434+
// so that parent node can be simplified further
435+
// e.g. (a() && false) && b() -> (a(), false) && b()
436+
// -> (a(), false && b()) -> (a(), false)
437+
return new JMultiExpression(info, lhs, rhs);
438+
} else {
434439
return rhs;
435440
}
436441
}
@@ -482,7 +487,9 @@ public static JExpression simplifyOr(JBinaryOperation expression) {
482487
if (rhs instanceof JBooleanLiteral) {
483488
if (!((JBooleanLiteral) rhs).getValue()) {
484489
return lhs;
485-
} else if (!lhs.hasSideEffects()) {
490+
} else if (lhs.hasSideEffects()) {
491+
return new JMultiExpression(info, lhs, rhs);
492+
} else {
486493
return rhs;
487494
}
488495
}

dev/core/test/com/google/gwt/dev/jjs/impl/DeadCodeEliminationTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,10 @@ public void testCommuteMultiExpression() throws Exception {
298298
+ "static int f1;"
299299
+ "static A createA() { A.f1 = 1; return new A(); } "
300300
+ "static boolean booleanWithSideEffects() { createA(); return true;}"
301+
+ "@javaemul.internal.annotations.DoNotInline "
302+
+ "static boolean notInlinedBool1() { return true;}"
303+
+ "@javaemul.internal.annotations.DoNotInline "
304+
+ "static boolean notInlinedBool2() { return true;}"
301305
+ "static boolean booleanWithoutSideEffects() { return true;}"
302306
+ "static int arithmeticWithSideEffects() { createA(); return 4;}"
303307
+ "}");
@@ -311,6 +315,28 @@ public void testCommuteMultiExpression() throws Exception {
311315
optimizeExpressions(false, "boolean", "false && A.booleanWithSideEffects()")
312316
.intoString("return false;");
313317

318+
optimizeExpressions(false, "boolean", "A.notInlinedBool1() && false")
319+
.intoString("return (EntryPoint$A.notInlinedBool1(), false);");
320+
321+
optimizeExpressions(false, "int",
322+
"A.notInlinedBool1() && (false && A.notInlinedBool2()) ? 1 : 2")
323+
.intoString("return (EntryPoint$A.notInlinedBool1(), 2);");
324+
325+
optimizeExpressions(false, "int",
326+
"(A.notInlinedBool1() && false) && A.notInlinedBool2() ? 1 : 2")
327+
.intoString("return (EntryPoint$A.notInlinedBool1(), 2);");
328+
329+
optimizeExpressions(false, "boolean", "A.notInlinedBool1() || true")
330+
.intoString("return (EntryPoint$A.notInlinedBool1(), true);");
331+
332+
optimizeExpressions(false, "int",
333+
"A.notInlinedBool1() || (true || A.notInlinedBool2()) ? 1 : 2")
334+
.intoString("return (EntryPoint$A.notInlinedBool1(), 1);");
335+
336+
optimizeExpressions(false, "int",
337+
"(A.notInlinedBool1() || true) || A.notInlinedBool2() ? 1 : 2")
338+
.intoString("return (EntryPoint$A.notInlinedBool1(), 1);");
339+
314340
optimizeExpressions(false, "int", "3 + A.arithmeticWithSideEffects()")
315341
.intoString("return (EntryPoint$A.f1 = 1, new EntryPoint$A(), 7);");
316342
}

0 commit comments

Comments
 (0)