Bug 829277: Limit the number of instructions that the truncation optimization can work on. (r=dvander)
authorMarty Rosenberg <mrosenberg@mozilla.com>
Thu, 10 Jan 2013 18:06:05 -0500
changeset 118481 2983ba3b514fd540f2f4c57b9fcd1dfe8338f7a8
parent 118480 855363cef32492e35e2f0ffbc608135c905dbc38
child 118482 f9134833700b4587ce6affb1479d75084f0020b7
push id24166
push userMs2ger@gmail.com
push dateFri, 11 Jan 2013 13:57:41 +0000
treeherdermozilla-central@63c4b0f66a0c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs829277
milestone21.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 829277: Limit the number of instructions that the truncation optimization can work on. (r=dvander)
js/src/ion/EdgeCaseAnalysis.cpp
js/src/ion/EdgeCaseAnalysis.h
js/src/ion/MIR.cpp
js/src/ion/MIR.h
--- a/js/src/ion/EdgeCaseAnalysis.cpp
+++ b/js/src/ion/EdgeCaseAnalysis.cpp
@@ -56,19 +56,22 @@ EdgeCaseAnalysis::analyzeEarly()
             return false;
         for (MInstructionReverseIterator riter(block->rbegin()); riter != block->rend(); riter++)
             riter->analyzeTruncateBackward();
     }
 
     return true;
 }
 
-bool
+int
 EdgeCaseAnalysis::AllUsesTruncate(MInstruction *m)
 {
+    // If all uses truncate, the return value must be at least 1. If anything doesn't truncate
+    // 0 is explicitly returned.
+    int ret = 1;
     for (MUseIterator use = m->usesBegin(); use != m->usesEnd(); use++) {
         // See #809485 why this is allowed
         if (use->node()->isResumePoint())
             continue;
 
         MDefinition *def = use->node()->toDefinition();
         if (def->isTruncateToInt32())
             continue;
@@ -79,17 +82,21 @@ EdgeCaseAnalysis::AllUsesTruncate(MInstr
         if (def->isBitXor())
             continue;
         if (def->isLsh())
             continue;
         if (def->isRsh())
             continue;
         if (def->isBitNot())
             continue;
-        if (def->isAdd() && def->toAdd()->isTruncated())
+        if (def->isAdd() && def->toAdd()->isTruncated()) {
+            ret = Max(ret, def->toAdd()->isTruncated() + 1);
             continue;
-        if (def->isSub() && def->toSub()->isTruncated())
+        }
+        if (def->isSub() && def->toSub()->isTruncated()) {
+            ret = Max(ret, def->toSub()->isTruncated() + 1);
             continue;
+        }
         // cannot use divide, since |truncate(int32(x/y) + int32(a/b)) != truncate(x/y+a/b)|
-        return false;
+        return 0;
     }
-    return true;
+    return ret;
 }
--- a/js/src/ion/EdgeCaseAnalysis.h
+++ b/js/src/ion/EdgeCaseAnalysis.h
@@ -17,17 +17,17 @@ class EdgeCaseAnalysis
 {
     MIRGenerator *mir;
     MIRGraph &graph;
 
   public:
     EdgeCaseAnalysis(MIRGenerator *mir, MIRGraph &graph);
     bool analyzeEarly();
     bool analyzeLate();
-    static bool AllUsesTruncate(MInstruction *m);
+    static int AllUsesTruncate(MInstruction *m);
 };
 
 
 } // namespace ion
 } // namespace js
 
 #endif // jsion_ion_edge_case_analysis_h__
 
--- a/js/src/ion/MIR.cpp
+++ b/js/src/ion/MIR.cpp
@@ -813,28 +813,29 @@ MDiv::analyzeEdgeCasesBackward()
 {
     if (canBeNegativeZero() && !NeedNegativeZeroCheck(this))
         setCanBeNegativeZero(false);
 }
 
 void
 MDiv::analyzeTruncateBackward()
 {
-    if (!isTruncated() && js::ion::EdgeCaseAnalysis::AllUsesTruncate(this))
-        setTruncated(true);
+    if (!isTruncated())
+        setTruncated(js::ion::EdgeCaseAnalysis::AllUsesTruncate(this));
 }
 
 bool
 MDiv::updateForReplacement(MDefinition *ins_)
 {
     JS_ASSERT(ins_->isDiv());
     MDiv *ins = ins_->toDiv();
     // Since EdgeCaseAnalysis is not being run before GVN, its information does
     // not need to be merged here.
-    setTruncated(isTruncated() && ins->isTruncated());
+    if (isTruncated())
+        setTruncated(Max(isTruncated(), ins->isTruncated()));
     return true;
 }
 
 static inline MDefinition *
 TryFold(MDefinition *original, MDefinition *replacement)
 {
     if (original->type() == replacement->type())
         return replacement;
@@ -851,55 +852,65 @@ MMod::foldsTo(bool useValueNumbers)
         return folded;
 
     return this;
 }
 
 void
 MAdd::analyzeTruncateBackward()
 {
-    if (!isTruncated() && js::ion::EdgeCaseAnalysis::AllUsesTruncate(this))
-        setTruncated(true);
+    if (!isTruncated())
+        setTruncated(js::ion::EdgeCaseAnalysis::AllUsesTruncate(this));
 }
 
 bool
 MAdd::updateForReplacement(MDefinition *ins_)
 {
     JS_ASSERT(ins_->isAdd());
     MAdd *ins = ins_->toAdd();
-    setTruncated(isTruncated() && ins->isTruncated());
+    if (isTruncated())
+        setTruncated(Max(isTruncated(), ins->isTruncated()));
     return true;
 }
 
 bool
 MAdd::fallible()
 {
-    return !isTruncated() && (!range() || !range()->isFinite());
+    // the add is fallible if range analysis does not say that it is finite, AND
+    // either the truncation analysis shows that there are non-truncated uses, or
+    // there are more than 20 operations before it gets truncated. 20 was chosen
+    // for two reasons. First, it is a nice sane number. Second, the largest int32
+    // can be (about) 2^31. The smallest integer that cannot be exactly represented
+    // as a double is 2^53 + 1  by doing something simple, like x = x + x, it takes
+    // 23 additions toget from 2^31 to 2^53 + 1. 20 is simply a conservative estimate of that.
+    return (!isTruncated() || isTruncated() > 20) && (!range() || !range()->isFinite());
 }
 
 void
 MSub::analyzeTruncateBackward()
 {
-    if (!isTruncated() && js::ion::EdgeCaseAnalysis::AllUsesTruncate(this))
-        setTruncated(true);
+    if (!isTruncated())
+        setTruncated(js::ion::EdgeCaseAnalysis::AllUsesTruncate(this));
 }
 
 bool
 MSub::updateForReplacement(MDefinition *ins_)
 {
     JS_ASSERT(ins_->isSub());
     MSub *ins = ins_->toSub();
-    setTruncated(isTruncated() && ins->isTruncated());
+    if (isTruncated())
+        setTruncated(Max(isTruncated(), ins->isTruncated()));
     return true;
 }
 
 bool
 MSub::fallible()
 {
-    return !isTruncated() && (!range() || !range()->isFinite());
+    // see comment in MAdd::fallible()
+    return (!isTruncated() || isTruncated() > 20) && (!range() || !range()->isFinite());
 }
 
 MDefinition *
 MMul::foldsTo(bool useValueNumbers)
 {
     MDefinition *out = MBinaryArithInstruction::foldsTo(useValueNumbers);
     if (out != this)
         return out;
--- a/js/src/ion/MIR.h
+++ b/js/src/ion/MIR.h
@@ -2554,68 +2554,68 @@ class MMathFunction
 
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
 };
 
 class MAdd : public MBinaryArithInstruction
 {
-    bool implicitTruncate_;
+    int implicitTruncate_;
 
     MAdd(MDefinition *left, MDefinition *right)
       : MBinaryArithInstruction(left, right),
-        implicitTruncate_(false)
+        implicitTruncate_(0)
     {
         setResultType(MIRType_Value);
     }
 
   public:
     INSTRUCTION_HEADER(Add)
     static MAdd *New(MDefinition *left, MDefinition *right) {
         return new MAdd(left, right);
     }
     void analyzeTruncateBackward();
 
-    bool isTruncated() const {
+    int isTruncated() const {
         return implicitTruncate_;
     }
-    void setTruncated(bool truncate) {
+    void setTruncated(int truncate) {
         implicitTruncate_ = truncate;
     }
     bool updateForReplacement(MDefinition *ins);
     double getIdentity() {
         return 0;
     }
 
     bool fallible();
     void computeRange();
 };
 
 class MSub : public MBinaryArithInstruction
 {
-    bool implicitTruncate_;
+    int implicitTruncate_;
     MSub(MDefinition *left, MDefinition *right)
       : MBinaryArithInstruction(left, right),
         implicitTruncate_(false)
     {
         setResultType(MIRType_Value);
     }
 
   public:
     INSTRUCTION_HEADER(Sub)
     static MSub *New(MDefinition *left, MDefinition *right) {
         return new MSub(left, right);
     }
 
     void analyzeTruncateBackward();
-    bool isTruncated() const {
+    int isTruncated() const {
         return implicitTruncate_;
     }
-    void setTruncated(bool truncate) {
+    void setTruncated(int truncate) {
         implicitTruncate_ = truncate;
     }
     bool updateForReplacement(MDefinition *ins);
 
     double getIdentity() {
         return 0;
     }
 
@@ -2720,17 +2720,17 @@ class MMul : public MBinaryArithInstruct
     Mode mode() { return mode_; }
 };
 
 class MDiv : public MBinaryArithInstruction
 {
     bool canBeNegativeZero_;
     bool canBeNegativeOverflow_;
     bool canBeDivideByZero_;
-    bool implicitTruncate_;
+    int implicitTruncate_;
 
     MDiv(MDefinition *left, MDefinition *right, MIRType type)
       : MBinaryArithInstruction(left, right),
         canBeNegativeZero_(true),
         canBeNegativeOverflow_(true),
         canBeDivideByZero_(true),
         implicitTruncate_(false)
     {
@@ -2754,20 +2754,20 @@ class MDiv : public MBinaryArithInstruct
     void analyzeEdgeCasesBackward();
     void analyzeTruncateBackward();
 
     double getIdentity() {
         JS_NOT_REACHED("not used");
         return 1;
     }
 
-    bool isTruncated() const {
+    int isTruncated() const {
         return implicitTruncate_;
     }
-    void setTruncated(bool truncate) {
+    void setTruncated(int truncate) {
         implicitTruncate_ = truncate;
     }
 
     bool canBeNegativeZero() {
         return canBeNegativeZero_;
     }
     void setCanBeNegativeZero(bool negativeZero) {
         canBeNegativeZero_ = negativeZero;