Bug 923659 - IonMonkey: Use the Range constructor which takes an MDefinition instead of calling range() directly to determine operand ranges. r=nbp
authorDan Gohman <sunfish@google.com>
Tue, 15 Oct 2013 20:49:43 -0700
changeset 164682 465f94eb495238c72b56ab54e17845e144a28efe
parent 164681 21f1754f307bbcb16847cc296ca2665a70d94b46
child 164683 ea7391a5932319227cafe292122a99e6ae08206a
push id3066
push userakeybl@mozilla.com
push dateMon, 09 Dec 2013 19:58:46 +0000
treeherdermozilla-beta@a31a0dce83aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs923659
milestone27.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
js/src/jit/RangeAnalysis.cpp
js/src/jit/RangeAnalysis.h
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -354,16 +354,20 @@ class MDefinition : public MNode
     void setTrackedPc(jsbytecode *pc) {
         trackedPc_ = pc;
     }
 
     jsbytecode *trackedPc() {
         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
     // errors. Instead, one should define the collectRangeInfo() to set the
     // right set of flags which are dependent on the range of the inputs.
     Range *range() const {
         JS_ASSERT(type() != MIRType_None);
         return range_;
@@ -433,16 +437,22 @@ class MDefinition : public MNode
     }\
     void set##flag##Unchecked() {\
         setFlags(1 << flag);\
     }
 
     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_;
     }
 
     types::TemporaryTypeSet *resultTypeSet() const {
         return resultTypeSet_;
     }
     bool emptyResultTypeSet() const;
--- a/js/src/jit/RangeAnalysis.cpp
+++ b/js/src/jit/RangeAnalysis.cpp
@@ -833,38 +833,39 @@ MPhi::computeRange()
         if (getOperand(i)->block()->earlyAbort()) {
             IonSpew(IonSpew_Range, "Ignoring unreachable input %d", getOperand(i)->id());
             continue;
         }
 
         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);
 }
 
 void
 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();
     } else {
         setRange(range);
     }
 }
 
@@ -2151,17 +2152,17 @@ AllUsesTruncate(MInstruction *candidate)
 
     return true;
 }
 
 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();
         if (!def->isTruncateToInt32() || !def->isToInt32())
             continue;
 
         def->replaceAllUsesWith(truncated);
     }
@@ -2221,17 +2222,19 @@ RangeAnalysis::truncate()
               case MDefinition::Op_Rsh:
               case MDefinition::Op_Ursh:
                 if (!bitops.append(static_cast<MBinaryBitwiseInstruction*>(*iter)))
                     return false;
               default:;
             }
 
             // 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();
 
             // Special case integer division: the result of a/b can be infinite
             // but cannot actually have rounding errors induced by truncation.
             if (iter->isDiv() && iter->toDiv()->specialization() == MIRType_Int32)
                 canHaveRoundingErrors = false;
 
@@ -2294,30 +2297,32 @@ RangeAnalysis::truncate()
 
 ///////////////////////////////////////////////////////////////////////////////
 // Collect Range information of operands
 ///////////////////////////////////////////////////////////////////////////////
 
 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_;
 }
--- a/js/src/jit/RangeAnalysis.h
+++ b/js/src/jit/RangeAnalysis.h
@@ -302,16 +302,19 @@ class Range : public TempObject {
         canHaveFractionalPart_(other.canHaveFractionalPart_),
         max_exponent_(other.max_exponent_),
         symbolicLower_(nullptr),
         symbolicUpper_(nullptr)
     {
         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) {
         return new Range(l, h, false, MaxInt32Exponent);
     }
 
     static Range *NewUInt32Range(uint32_t l, uint32_t h) {
         // For now, just pass them to the constructor as int64_t values.