Bug 1319242 - ExtractLinearSum should remain within the same arithmetic space. r=jandem
authorNicolas B. Pierron <nicolas.b.pierron@mozilla.com>
Wed, 23 Nov 2016 15:24:09 +0000
changeset 324097 9789423a63aba19ca6b5fecb97f4dacbb1fc0181
parent 324096 5f23873d0c91d0d2d647166d7d6491d388f4825e
child 324098 5032b5290d7cd1ac6fcbb0ef57311771ccdfad4b
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersjandem
bugs1319242
milestone53.0a1
Bug 1319242 - ExtractLinearSum should remain within the same arithmetic space. r=jandem
js/src/jit-test/tests/ion/fold-linear-arith-bug1319242.js
js/src/jit/IonAnalysis.cpp
js/src/jit/IonAnalysis.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/fold-linear-arith-bug1319242.js
@@ -0,0 +1,7 @@
+function f(x) {
+    // Check that we do not fold +1 and -2 across truncated/non-truncated operations.
+    return (((x | 0) + 1) | 0) + -2;
+}
+const int32_min = -Math.pow(2,31);
+f(Infinity);
+assertEq(f(int32_min - 1), int32_min - 2);
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -3094,55 +3094,98 @@ FindDominatingBoundsCheck(BoundsCheckMap
             return nullptr;
 
         return check;
     }
 
     return p->value().check;
 }
 
+static MathSpace
+ExtractMathSpace(MDefinition* ins)
+{
+    MOZ_ASSERT(ins->isAdd() || ins->isSub());
+    MBinaryArithInstruction* arith = nullptr;
+    if (ins->isAdd())
+        arith = ins->toAdd();
+    else
+        arith = ins->toSub();
+    switch (arith->truncateKind()) {
+      case MDefinition::NoTruncate:
+      case MDefinition::TruncateAfterBailouts:
+        // TruncateAfterBailouts is considered as infinite space because the
+        // LinearSum will effectively remove the bailout check.
+        return MathSpace::Infinite;
+      case MDefinition::IndirectTruncate:
+      case MDefinition::Truncate:
+        return MathSpace::Modulo;
+    }
+    MOZ_MAKE_COMPILER_ASSUME_IS_UNREACHABLE("Unknown TruncateKind");
+}
+
 // Extract a linear sum from ins, if possible (otherwise giving the sum 'ins + 0').
 SimpleLinearSum
-jit::ExtractLinearSum(MDefinition* ins)
+jit::ExtractLinearSum(MDefinition* ins, MathSpace space)
 {
     if (ins->isBeta())
         ins = ins->getOperand(0);
 
     if (ins->type() != MIRType::Int32)
         return SimpleLinearSum(ins, 0);
 
     if (ins->isConstant())
         return SimpleLinearSum(nullptr, ins->toConstant()->toInt32());
 
-    if (ins->isAdd() || ins->isSub()) {
-        MDefinition* lhs = ins->getOperand(0);
-        MDefinition* rhs = ins->getOperand(1);
-        if (lhs->type() == MIRType::Int32 && rhs->type() == MIRType::Int32) {
-            SimpleLinearSum lsum = ExtractLinearSum(lhs);
-            SimpleLinearSum rsum = ExtractLinearSum(rhs);
-
-            if (lsum.term && rsum.term)
-                return SimpleLinearSum(ins, 0);
-
-            // Check if this is of the form <SUM> + n, n + <SUM> or <SUM> - n.
-            if (ins->isAdd()) {
-                int32_t constant;
-                if (!SafeAdd(lsum.constant, rsum.constant, &constant))
-                    return SimpleLinearSum(ins, 0);
-                return SimpleLinearSum(lsum.term ? lsum.term : rsum.term, constant);
-            }
-            if (lsum.term) {
-                int32_t constant;
-                if (!SafeSub(lsum.constant, rsum.constant, &constant))
-                    return SimpleLinearSum(ins, 0);
-                return SimpleLinearSum(lsum.term, constant);
-            }
-        }
+    if (!ins->isAdd() && !ins->isSub())
+        return SimpleLinearSum(ins, 0);
+
+    // Only allow math which are in the same space.
+    MathSpace insSpace = ExtractMathSpace(ins);
+    if (space == MathSpace::Unknown)
+        space = insSpace;
+    else if (space != insSpace)
+        return SimpleLinearSum(ins, 0);
+    MOZ_ASSERT(space == MathSpace::Modulo || space == MathSpace::Infinite);
+
+    MDefinition* lhs = ins->getOperand(0);
+    MDefinition* rhs = ins->getOperand(1);
+    if (lhs->type() != MIRType::Int32 || rhs->type() != MIRType::Int32)
+        return SimpleLinearSum(ins, 0);
+
+    // Extract linear sums of each operand.
+    SimpleLinearSum lsum = ExtractLinearSum(lhs, space);
+    SimpleLinearSum rsum = ExtractLinearSum(rhs, space);
+
+    // LinearSum only considers a single term operand, if both sides have
+    // terms, then ignore extracted linear sums.
+    if (lsum.term && rsum.term)
+        return SimpleLinearSum(ins, 0);
+
+    // Check if this is of the form <SUM> + n or n + <SUM>.
+    if (ins->isAdd()) {
+        int32_t constant;
+        if (space == MathSpace::Modulo)
+            constant = lsum.constant + rsum.constant;
+        else if (!SafeAdd(lsum.constant, rsum.constant, &constant))
+            return SimpleLinearSum(ins, 0);
+        return SimpleLinearSum(lsum.term ? lsum.term : rsum.term, constant);
     }
 
+    MOZ_ASSERT(ins->isSub());
+    // Check if this is of the form <SUM> - n.
+    if (lsum.term) {
+        int32_t constant;
+        if (space == MathSpace::Modulo)
+            constant = lsum.constant - rsum.constant;
+        else if (!SafeSub(lsum.constant, rsum.constant, &constant))
+            return SimpleLinearSum(ins, 0);
+        return SimpleLinearSum(lsum.term, constant);
+    }
+
+    // Ignore any of the form n - <SUM>.
     return SimpleLinearSum(ins, 0);
 }
 
 // Extract a linear inequality holding when a boolean test goes in the
 // specified direction, of the form 'lhs + lhsN <= rhs' (or >=).
 bool
 jit::ExtractLinearInequality(MTest* test, BranchDirection direction,
                              SimpleLinearSum* plhs, MDefinition** prhs, bool* plessEqual)
--- a/js/src/jit/IonAnalysis.h
+++ b/js/src/jit/IonAnalysis.h
@@ -104,18 +104,31 @@ struct SimpleLinearSum
     MDefinition* term;
     int32_t constant;
 
     SimpleLinearSum(MDefinition* term, int32_t constant)
         : term(term), constant(constant)
     {}
 };
 
+// Math done in a Linear sum can either be in a modulo space, in which case
+// overflow are wrapped around, or they can be computed in the integer-space in
+// which case we have to check that no overflow can happen when summing
+// constants.
+//
+// When the caller ignores which space it is, the definition would be used to
+// deduce it.
+enum class MathSpace {
+    Modulo,
+    Infinite,
+    Unknown
+};
+
 SimpleLinearSum
-ExtractLinearSum(MDefinition* ins);
+ExtractLinearSum(MDefinition* ins, MathSpace space = MathSpace::Unknown);
 
 MOZ_MUST_USE bool
 ExtractLinearInequality(MTest* test, BranchDirection direction,
                         SimpleLinearSum* plhs, MDefinition** prhs, bool* plessEqual);
 
 struct LinearTerm
 {
     MDefinition* term;