Skip to content

Commit

Permalink
Handle algebraic identities in PeepholeFoldConstants
Browse files Browse the repository at this point in the history
In particular:
x*1 -> x
x-0 -> x
0-x -> -x
x/1 -> x
x+0 -> x

An upcoming change to the Soy compiler will generate some of these trivial
expressions, and while it would be easy to work around on our end it seemed
like a reasonable feature for the jscompiler to handle.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=179462284
  • Loading branch information
lukesandberg authored and Tyler Breisacher committed Dec 21, 2017
1 parent d6a9fd1 commit f3dff41
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 22 deletions.
93 changes: 72 additions & 21 deletions src/com/google/javascript/jscomp/PeepholeFoldConstants.java
Expand Up @@ -740,17 +740,75 @@ private Node performArithmeticOp(Token opType, Node left, Node right) {
// to zero so this is a little awkward here.

Double lValObj = NodeUtil.getNumberValue(left);
if (lValObj == null) {
Double rValObj = NodeUtil.getNumberValue(right);
// at least one of the two operands must have a value and both must be numeric
if ((lValObj == null && rValObj == null) || !isNumeric(left) || !isNumeric(right)) {
return null;
}
Double rValObj = NodeUtil.getNumberValue(right);
if (rValObj == null) {
// handle the operations that have algebraic identities, since we can simplify the tree without
// actually knowing the value statically.
switch (opType) {
case ADD:
if (lValObj != null && rValObj != null) {
return maybeReplaceBinaryOpWithNumericResult(lValObj + rValObj, lValObj, rValObj);
}
if (lValObj != null && lValObj == 0) {
return right.cloneTree(true);
} else if (rValObj != null && rValObj == 0) {
return left.cloneTree(true);
}
return null;
case SUB:
if (lValObj != null && rValObj != null) {
return maybeReplaceBinaryOpWithNumericResult(lValObj - rValObj, lValObj, rValObj);
}
if (lValObj != null && lValObj == 0) {
// 0 - x -> -x
return IR.neg(right.cloneTree(true));
} else if (rValObj != null && rValObj == 0) {
// x - 0 -> x
return left.cloneTree(true);
}
return null;
case MUL:
if (lValObj != null && rValObj != null) {
return maybeReplaceBinaryOpWithNumericResult(lValObj * rValObj, lValObj, rValObj);
}
// NOTE: 0*x != 0 for all x, if x==0, then it is NaN. So we can't take advantage of that
// without some kind of non-NaN proof. So the special cases here only deal with 1*x
if (lValObj != null) {
if (lValObj == 1) {
return right.cloneTree(true);
}
} else {
if (rValObj == 1) {
return left.cloneTree(true);
}
}
return null;
case DIV:
if (lValObj != null && rValObj != null) {
if (rValObj == 0) {
return null;
}
return maybeReplaceBinaryOpWithNumericResult(lValObj / rValObj, lValObj, rValObj);
}
// NOTE: 0/x != 0 for all x, if x==0, then it is NaN
if (rValObj != null) {
if (rValObj == 1) {
// x/1->x
return left.cloneTree(true);
}
}
return null;
default:
// fall-through
}
if (lValObj == null || rValObj == null) {
return null;
}

double lval = lValObj;
double rval = rValObj;

switch (opType) {
case BITAND:
result = NodeUtil.toInt32(lval) & NodeUtil.toInt32(rval);
Expand All @@ -761,31 +819,24 @@ private Node performArithmeticOp(Token opType, Node left, Node right) {
case BITXOR:
result = NodeUtil.toInt32(lval) ^ NodeUtil.toInt32(rval);
break;
case ADD:
result = lval + rval;
break;
case SUB:
result = lval - rval;
break;
case MUL:
result = lval * rval;
break;
case MOD:
if (rval == 0) {
return null;
}
result = lval % rval;
break;
case DIV:
if (rval == 0) {
return null;
}
result = lval / rval;
break;
default:
throw new Error("Unexpected arithmetic operator");
throw new Error("Unexpected arithmetic operator: " + opType);
}
return maybeReplaceBinaryOpWithNumericResult(result, lval, rval);
}

private boolean isNumeric(Node n) {
return NodeUtil.isNumericResult(n)
|| (shouldUseTypes && n.getTypeI() != null && n.getTypeI().isNumberValueType());
}

private Node maybeReplaceBinaryOpWithNumericResult(double result, double lval, double rval) {
// TODO(johnlenz): consider removing the result length check.
// length of the left and right value plus 1 byte for the operator.
if ((String.valueOf(result).length() <=
Expand Down
32 changes: 31 additions & 1 deletion test/com/google/javascript/jscomp/PeepholeFoldConstantsTest.java
Expand Up @@ -1199,7 +1199,7 @@ public void testFoldLeftChildOp() {
foldSame("1 + x - 2 + 3");
foldSame("1 + x - 2 + 3 - 1");
foldSame("f(x)-0");
foldSame("x-0-0");
fold("x-0-0", "x-0");
foldSame("x+2-2+2");
foldSame("x+2-2+2-2");
foldSame("x-2+2");
Expand Down Expand Up @@ -1507,6 +1507,36 @@ public void testConvertToNumberNegativeInf() {
foldSame("var x = 3 * (r ? Infinity : -Infinity);");
}

public void testAlgebraicIdentities() {
this.mode = TypeInferenceMode.BOTH;

foldNumericTypes("x+0", "x");
foldNumericTypes("0+x", "x");
foldNumericTypes("x+0+0+x+x+0", "x+x+x");

foldNumericTypes("x-0", "x");
foldNumericTypes("x-0-0-0", "x");
// 'x-0' is numeric even if x isn't
fold("var x='a'; x-0-0", "var x='a';x-0");
foldNumericTypes("0-x", "-x");
fold("for (var i = 0; i < 5; i++) var x = 0 + i * 1", "for(var i=0; i < 5; i++) var x=i");

foldNumericTypes("x*1", "x");
foldNumericTypes("1*x", "x");
// can't optimize these without a non-NaN prover
foldSame("x*0");
foldSame("0*x");
foldSame("0/x");

foldNumericTypes("x/1", "x");
}

private void foldNumericTypes(String js, String expected) {
fold(
"function f(/** @type {number} */ x) { " + js + " }",
"function f(/** @type {number} */ x) { " + expected + " }");
}

private static String join(String operandA, String op, String operandB) {
return operandA + " " + op + " " + operandB;
}
Expand Down

0 comments on commit f3dff41

Please sign in to comment.