Fix for issue #851; implements some dead code eliminations suggested by sandymac.
Suggested by: sandymac
Review by: mmendez, alex.tkachman
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@983 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
index aad50fd..e2cd025 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
@@ -85,9 +85,12 @@
} else if (rhs instanceof JBooleanLiteral) {
// eg: if (isWhatever() && true) -> if (isWhatever())
+ // eg: if (isWhatever() && false) -> if (false), unless side effects
JBooleanLiteral booleanLiteral = (JBooleanLiteral) rhs;
if (booleanLiteral.getValue()) {
ctx.replaceMe(lhs);
+ } else if (!lhs.hasSideEffects()) {
+ ctx.replaceMe(rhs);
}
}
@@ -105,9 +108,12 @@
} else if (rhs instanceof JBooleanLiteral) {
// eg: if (isWhatever() || false) -> if (isWhatever())
+ // eg: if (isWhatever() && true) -> if (true), unless side effects
JBooleanLiteral booleanLiteral = (JBooleanLiteral) rhs;
if (!booleanLiteral.getValue()) {
ctx.replaceMe(lhs);
+ } else if (!lhs.hasSideEffects()) {
+ ctx.replaceMe(rhs);
}
}
} else if (op == JBinaryOperator.EQ) {
@@ -146,16 +152,48 @@
}
public void endVisit(JConditional x, Context ctx) {
- JExpression expression = x.getIfTest();
- if (expression instanceof JBooleanLiteral) {
- JBooleanLiteral booleanLiteral = (JBooleanLiteral) expression;
-
- if (booleanLiteral.getValue()) {
- // If true, replace myself with then expression
- ctx.replaceMe(x.getThenExpr());
+ JExpression condExpr = x.getIfTest();
+ JExpression thenExpr = x.getThenExpr();
+ JExpression elseExpr = x.getElseExpr();
+ if (condExpr instanceof JBooleanLiteral) {
+ if (((JBooleanLiteral) condExpr).getValue()) {
+ // e.g. (true ? then : else) -> then
+ ctx.replaceMe(thenExpr);
} else {
- // If false, replace myself with else expression
- ctx.replaceMe(x.getElseExpr());
+ // e.g. (false ? then : else) -> else
+ ctx.replaceMe(elseExpr);
+ }
+ } else if (thenExpr instanceof JBooleanLiteral) {
+ if (((JBooleanLiteral) thenExpr).getValue()) {
+ // e.g. (cond ? true : else) -> cond || else
+ JBinaryOperation binOp = new JBinaryOperation(program,
+ x.getSourceInfo(), x.getType(), JBinaryOperator.OR, condExpr,
+ elseExpr);
+ ctx.replaceMe(binOp);
+ } else {
+ // e.g. (cond ? false : else) -> !cond && else
+ JPrefixOperation notCondExpr = new JPrefixOperation(program,
+ condExpr.getSourceInfo(), JUnaryOperator.NOT, condExpr);
+ JBinaryOperation binOp = new JBinaryOperation(program,
+ x.getSourceInfo(), x.getType(), JBinaryOperator.AND, notCondExpr,
+ elseExpr);
+ ctx.replaceMe(binOp);
+ }
+ } else if (elseExpr instanceof JBooleanLiteral) {
+ if (((JBooleanLiteral) elseExpr).getValue()) {
+ // e.g. (cond ? then : true) -> !cond || then
+ JPrefixOperation notCondExpr = new JPrefixOperation(program,
+ condExpr.getSourceInfo(), JUnaryOperator.NOT, condExpr);
+ JBinaryOperation binOp = new JBinaryOperation(program,
+ x.getSourceInfo(), x.getType(), JBinaryOperator.OR, notCondExpr,
+ thenExpr);
+ ctx.replaceMe(binOp);
+ } else {
+ // e.g. (cond ? then : false) -> cond && then
+ JBinaryOperation binOp = new JBinaryOperation(program,
+ x.getSourceInfo(), x.getType(), JBinaryOperator.AND, condExpr,
+ thenExpr);
+ ctx.replaceMe(binOp);
}
}
}
@@ -229,13 +267,53 @@
}
/**
- * Resolve "!true" into "false" and vice versa.
+ * Simplify the ! operator if possible.
*/
public void endVisit(JPrefixOperation x, Context ctx) {
if (x.getOp() == JUnaryOperator.NOT) {
- if (x.getArg() instanceof JBooleanLiteral) {
- JBooleanLiteral booleanLiteral = (JBooleanLiteral) x.getArg();
+ JExpression arg = x.getArg();
+ if (arg instanceof JBooleanLiteral) {
+ // e.g. !true -> false; !false -> true
+ JBooleanLiteral booleanLiteral = (JBooleanLiteral) arg;
ctx.replaceMe(program.getLiteralBoolean(!booleanLiteral.getValue()));
+ } else if (arg instanceof JBinaryOperation) {
+ // try to invert the binary operator
+ JBinaryOperation argOp = (JBinaryOperation) arg;
+ JBinaryOperator op = argOp.getOp();
+ JBinaryOperator newOp = null;
+ if (op == JBinaryOperator.EQ) {
+ // e.g. !(x == y) -> x != y
+ newOp = JBinaryOperator.NEQ;
+ } else if (op == JBinaryOperator.NEQ) {
+ // e.g. !(x != y) -> x == y
+ newOp = JBinaryOperator.EQ;
+ } else if (op == JBinaryOperator.GT) {
+ // e.g. !(x > y) -> x <= y
+ newOp = JBinaryOperator.LTE;
+ } else if (op == JBinaryOperator.LTE) {
+ // e.g. !(x <= y) -> x > y
+ newOp = JBinaryOperator.GT;
+ } else if (op == JBinaryOperator.GTE) {
+ // e.g. !(x >= y) -> x < y
+ newOp = JBinaryOperator.LT;
+ } else if (op == JBinaryOperator.LT) {
+ // e.g. !(x < y) -> x >= y
+ newOp = JBinaryOperator.GTE;
+ }
+ if (newOp != null) {
+ JBinaryOperation newBinOp = new JBinaryOperation(program,
+ argOp.getSourceInfo(), argOp.getType(), newOp, argOp.getLhs(),
+ argOp.getRhs());
+ ctx.replaceMe(newBinOp);
+ }
+ } else if (arg instanceof JPrefixOperation) {
+ // try to invert the unary operator
+ JPrefixOperation argOp = (JPrefixOperation) arg;
+ JUnaryOperator op = argOp.getOp();
+ // e.g. !!x -> x
+ if (op == JUnaryOperator.NOT) {
+ ctx.replaceMe(argOp.getArg());
+ }
}
}
}
diff --git a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
index 1dde413..21466f8 100644
--- a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
+++ b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
@@ -90,16 +90,16 @@
}
}
+ private static class SideEffectCauser6 extends SideEffectCauser6Super {
+ public static String causeClinitSideEffectOnRead = "bar";
+ }
+
private static class SideEffectCauser6Super {
static {
CompilerTest.sideEffectChecker++;
}
}
- private static class SideEffectCauser6 extends SideEffectCauser6Super {
- public static String causeClinitSideEffectOnRead = "bar";
- }
-
private static final class UninstantiableType {
public Object field;
@@ -117,10 +117,6 @@
return "bar";
}
- private static native boolean cannotOptimize() /*-{
- return true;
- }-*/;
-
private static void foo(String string) {
Object o = string;
}
@@ -133,6 +129,14 @@
return @com.google.gwt.dev.jjs.test.CompilerTest$SideEffectCauser5::causeClinitSideEffectOnRead;
}-*/;
+ private static native boolean noOptimizeFalse() /*-{
+ return false;
+ }-*/;
+
+ private static native boolean noOptimizeTrue() /*-{
+ return true;
+ }-*/;
+
public String getModuleName() {
return "com.google.gwt.dev.jjs.CompilerSuite";
}
@@ -148,8 +152,8 @@
oaa[0][0] = "bar";
assertEquals(oaa[0][0], "bar");
- Apple[] apple = cannotOptimize() ? new Granny[3] : new Apple[3];
- Apple g = cannotOptimize() ? (Apple) new Granny() : (Apple) new Fuji();
+ Apple[] apple = noOptimizeTrue() ? new Granny[3] : new Apple[3];
+ Apple g = noOptimizeTrue() ? (Apple) new Granny() : (Apple) new Fuji();
Apple a = apple[0] = g;
assertEquals(g, a);
}
@@ -203,6 +207,28 @@
assertEquals(null, checkRescued);
}
+ public void testConditionals() {
+ assertTrue(noOptimizeTrue() ? noOptimizeTrue() : noOptimizeFalse());
+ assertFalse(noOptimizeFalse() ? noOptimizeTrue() : noOptimizeFalse());
+ assertFalse(noOptimizeTrue() ? noOptimizeFalse() : noOptimizeTrue());
+ assertTrue(noOptimizeFalse() ? noOptimizeFalse() : noOptimizeTrue());
+
+ assertTrue(true ? noOptimizeTrue() : noOptimizeFalse());
+ assertFalse(false ? noOptimizeTrue() : noOptimizeFalse());
+ assertFalse(true ? noOptimizeFalse() : noOptimizeTrue());
+ assertTrue(false ? noOptimizeFalse() : noOptimizeTrue());
+
+ assertTrue(noOptimizeTrue() ? true : noOptimizeFalse());
+ assertFalse(noOptimizeFalse() ? true : noOptimizeFalse());
+ assertFalse(noOptimizeTrue() ? false : noOptimizeTrue());
+ assertTrue(noOptimizeFalse() ? false : noOptimizeTrue());
+
+ assertTrue(noOptimizeTrue() ? noOptimizeTrue() : false);
+ assertFalse(noOptimizeFalse() ? noOptimizeTrue() : false);
+ assertFalse(noOptimizeTrue() ? noOptimizeFalse() : true);
+ assertTrue(noOptimizeFalse() ? noOptimizeFalse() : true);
+ }
+
public void testDeadCode() {
while (returnFalse()) {
break;
@@ -420,7 +446,7 @@
}
public void testJavaScriptReservedWords() {
- boolean delete = cannotOptimize();
+ boolean delete = noOptimizeTrue();
for (int in = 0; in < 10; ++in) {
assertTrue(in < 10);
assertTrue(delete);
@@ -480,8 +506,8 @@
}
public void testLocalRefs() {
- final String foo = cannotOptimize() ? "foo" : "bar";
- final String bar = cannotOptimize() ? "bar" : "foo";
+ final String foo = noOptimizeTrue() ? "foo" : "bar";
+ final String bar = noOptimizeTrue() ? "bar" : "foo";
String result = new Object() {
private String a = foo;
@@ -516,6 +542,27 @@
assertEquals(result, "foofoofoofoo");
}
+ public void testNotOptimizations() {
+ assertFalse(!true);
+ assertTrue(!false);
+
+ assertTrue(!(noOptimizeTrue() == noOptimizeFalse()));
+ assertFalse(!(noOptimizeTrue() != noOptimizeFalse()));
+
+ assertFalse(!(3 < 4));
+ assertFalse(!(3 <= 4));
+ assertTrue(!(3 > 4));
+ assertTrue(!(3 >= 4));
+
+ assertTrue(!(4 < 3));
+ assertTrue(!(4 <= 3));
+ assertFalse(!(4 > 3));
+ assertFalse(!(4 >= 3));
+
+ assertTrue(!!noOptimizeTrue());
+ assertFalse(!!noOptimizeFalse());
+ }
+
public void testNullFlow() {
UninstantiableType f = null;
