From 12f63606a47b93acf2be2c60af13249837282d31 Mon Sep 17 00:00:00 2001
From: Dan Gohman <sunfish@google.com>
Date: Tue, 15 Oct 2013 20:49:43 -0700
Subject: [PATCH] Bug 923659 - IonMonkey: Use the Range constructor which takes
 an MDefinition instead of calling range() directly to determine operand
 ranges. r=nbp

---
 js/src/jit/MIR.h             | 10 ++++++++++
 js/src/jit/RangeAnalysis.cpp | 37 ++++++++++++++++++++----------------
 js/src/jit/RangeAnalysis.h   |  3 +++
 3 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h
index 3419f806a89c..10e4fbd80898 100644
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -359,6 +359,10 @@ class MDefinition : public MNode
         return trackedPc_;
     }
 
+    // Return the range of this value, *before* any bailout checks. Contrast
+    // this with the type() method, and the Range constructor which takes an
+    // MDefinition*, which describe the value *after* any bailout checks.
+    //
     // Warning: Range analysis is removing the bit-operations such as '| 0' at
     // the end of the transformations. Using this function to analyse any
     // operands after the truncate phase of the range analysis will lead to
@@ -438,6 +442,12 @@ class MDefinition : public MNode
     MIR_FLAG_LIST(FLAG_ACCESSOR)
 #undef FLAG_ACCESSOR
 
+    // Return the type of this value. This may be speculative, and enforced
+    // dynamically with the use of bailout checks. If all the bailout checks
+    // pass, the value will have this type.
+    //
+    // Unless this is an MUrsh, which, as a special case, may return a value
+    // in (INT32_MAX,UINT32_MAX] even when its type() is MIRType_Int32.
     MIRType type() const {
         return resultType_;
     }
diff --git a/js/src/jit/RangeAnalysis.cpp b/js/src/jit/RangeAnalysis.cpp
index 5a8b85101865..aced78776210 100644
--- a/js/src/jit/RangeAnalysis.cpp
+++ b/js/src/jit/RangeAnalysis.cpp
@@ -838,17 +838,17 @@ MPhi::computeRange()
         if (isOSRLikeValue(getOperand(i)))
             continue;
 
-        Range *input = getOperand(i)->range();
+        // Peek at the pre-bailout range so we can take a short-cut; if any of
+        // the operands has an unknown range, this phi has an unknown range.
+        if (!getOperand(i)->range())
+            return;
 
-        if (!input) {
-            range = nullptr;
-            break;
-        }
+        Range input(getOperand(i));
 
         if (range)
-            range->unionWith(input);
+            range->unionWith(&input);
         else
-            range = new Range(*input);
+            range = new Range(input);
     }
 
     setRange(range);
@@ -859,7 +859,8 @@ MBeta::computeRange()
 {
     bool emptyRange = false;
 
-    Range *range = Range::intersect(getOperand(0)->range(), comparison_, &emptyRange);
+    Range opRange(getOperand(0));
+    Range *range = Range::intersect(&opRange, comparison_, &emptyRange);
     if (emptyRange) {
         IonSpew(IonSpew_Range, "Marking block for inst %d unexitable", id());
         block()->setEarlyAbort();
@@ -2156,7 +2157,7 @@ static void
 RemoveTruncatesOnOutput(MInstruction *truncated)
 {
     JS_ASSERT(truncated->type() == MIRType_Int32);
-    JS_ASSERT_IF(truncated->range(), truncated->range()->isInt32());
+    JS_ASSERT(Range(truncated).isInt32());
 
     for (MUseDefIterator use(truncated); use; use++) {
         MDefinition *def = use.def();
@@ -2226,7 +2227,9 @@ RangeAnalysis::truncate()
             }
 
             // Set truncated flag if range analysis ensure that it has no
-            // rounding errors and no fractional part.
+            // rounding errors and no fractional part. Note that we can't use
+            // the MDefinition Range constructor, because we need to know if
+            // the value will have rounding errors before any bailout checks.
             const Range *r = iter->range();
             bool canHaveRoundingErrors = !r || r->canHaveRoundingErrors();
 
@@ -2299,25 +2302,27 @@ RangeAnalysis::truncate()
 void
 MInArray::collectRangeInfo()
 {
-    needsNegativeIntCheck_ = !index()->range() || !index()->range()->isFiniteNonNegative();
+    Range indexRange(index());
+    needsNegativeIntCheck_ = !indexRange.isFiniteNonNegative();
 }
 
 void
 MLoadElementHole::collectRangeInfo()
 {
-    needsNegativeIntCheck_ = !index()->range() || !index()->range()->isFiniteNonNegative();
+    Range indexRange(index());
+    needsNegativeIntCheck_ = !indexRange.isFiniteNonNegative();
 }
 
 void
 MMod::collectRangeInfo()
 {
-    canBeNegativeDividend_ = !lhs()->range() || !lhs()->range()->isFiniteNonNegative();
+    Range lhsRange(lhs());
+    canBeNegativeDividend_ = !lhsRange.isFiniteNonNegative();
 }
 
 void
 MBoundsCheckLower::collectRangeInfo()
 {
-    fallible_ = !index()->range() ||
-                !index()->range()->hasInt32LowerBound() ||
-                index()->range()->lower() < minimum_;
+    Range indexRange(index());
+    fallible_ = !indexRange.hasInt32LowerBound() || indexRange.lower() < minimum_;
 }
diff --git a/js/src/jit/RangeAnalysis.h b/js/src/jit/RangeAnalysis.h
index cbe482e6b9f3..2c11b71a072c 100644
--- a/js/src/jit/RangeAnalysis.h
+++ b/js/src/jit/RangeAnalysis.h
@@ -307,6 +307,9 @@ class Range : public TempObject {
         assertInvariants();
     }
 
+    // Construct a range from the given MDefinition. This differs from the
+    // MDefinition's range() method in that it describes the range of values
+    // *after* any bailout checks.
     Range(const MDefinition *def);
 
     static Range *NewInt32Range(int32_t l, int32_t h) {