Bug 816787: Remove negative zero check for truncated uses, r=mjrosenb
authorHannes Verschore <hv1989@gmail.com>
Mon, 03 Dec 2012 00:09:56 +0100
changeset 114758 92c4255955d2b7c61ab23739fefe9e3fdf0254eb
parent 114757 9160d646fa8b9d492e3395bcd29343a84f59fdc4
child 114759 5497a1ba0f9131b5f80b80ba81a35d40795cab7f
push id23938
push useremorley@mozilla.com
push dateMon, 03 Dec 2012 19:05:29 +0000
treeherdermozilla-central@662eaec3b329 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmjrosenb
bugs816787
milestone20.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 816787: Remove negative zero check for truncated uses, r=mjrosenb
js/src/ion/MIR.cpp
js/src/ion/MIR.h
js/src/ion/RangeAnalysis.cpp
js/src/ion/RangeAnalysis.h
--- a/js/src/ion/MIR.cpp
+++ b/js/src/ion/MIR.cpp
@@ -583,24 +583,28 @@ MUrsh::infer(const TypeOracle::BinaryTyp
 
     specialization_ = MIRType_Int32;
     JS_ASSERT(type() == MIRType_Int32);
 }
 
 static inline bool
 NeedNegativeZeroCheck(MDefinition *def)
 {
-    // Test if all uses have the same symantic for -0 and 0
+    // Test if all uses have the same semantics for -0 and 0
     for (MUseIterator use = def->usesBegin(); use != def->usesEnd(); use++) {
         if (use->node()->isResumePoint())
             continue;
 
         MDefinition *use_def = use->node()->toDefinition();
         switch (use_def->op()) {
           case MDefinition::Op_Add: {
+            // If add is truncating -0 and 0 are observed as the same.
+            if (use_def->toAdd()->isTruncated())
+                break;
+
             // x + y gives -0, when both x and y are -0
 
             // Figure out the order in which the addition's operands will
             // execute. EdgeCaseAnalysis::analyzeLate has renumbered the MIR
             // definitions for us so that this just requires comparing ids.
             MDefinition *first = use_def->getOperand(0);
             MDefinition *second = use_def->getOperand(1);
             if (first->id() > second->id()) {
@@ -635,25 +639,29 @@ NeedNegativeZeroCheck(MDefinition *def)
                 }
             }
 
             // The negative zero check can always be removed on the second
             // executed operand; by the time this executes the first will have
             // been evaluated as int32 and the addition's result cannot be -0.
             break;
           }
+          case MDefinition::Op_Sub:
+            // If sub is truncating -0 and 0 are observed as the same
+            if (use_def->toSub()->isTruncated())
+                break;
+            /* Fall through...  */
           case MDefinition::Op_StoreElement:
           case MDefinition::Op_StoreElementHole:
           case MDefinition::Op_LoadElement:
           case MDefinition::Op_LoadElementHole:
           case MDefinition::Op_LoadTypedArrayElement:
           case MDefinition::Op_LoadTypedArrayElementHole:
           case MDefinition::Op_CharCodeAt:
           case MDefinition::Op_Mod:
-          case MDefinition::Op_Sub:
             // Only allowed to remove check when definition is the second operand
             if (use_def->getOperand(0) == def)
                 return true;
             if (use_def->numOperands() > 2) {
                 for (size_t i = 2; i < use_def->numOperands(); i++) {
                     if (use_def->getOperand(i) == def)
                         return true;
                 }
@@ -667,16 +675,17 @@ NeedNegativeZeroCheck(MDefinition *def)
           case MDefinition::Op_ToString:
           case MDefinition::Op_FromCharCode:
           case MDefinition::Op_TableSwitch:
           case MDefinition::Op_Compare:
           case MDefinition::Op_BitAnd:
           case MDefinition::Op_BitOr:
           case MDefinition::Op_BitXor:
           case MDefinition::Op_Abs:
+          case MDefinition::Op_TruncateToInt32:
             // Always allowed to remove check. No matter which operand.
             break;
           default:
             return true;
         }
     }
     return false;
 }
@@ -730,57 +739,56 @@ MDiv::analyzeEdgeCasesForward()
 
     // Try removing divide by zero check
     if (rhs()->isConstant() && !rhs()->toConstant()->value().isInt32(0))
         canBeDivideByZero_ =  false;
 
     // If lhs is a constant int != INT32_MIN, then
     // negative overflow check can be skipped.
     if (lhs()->isConstant() && !lhs()->toConstant()->value().isInt32(INT32_MIN))
-        canBeNegativeOverflow_ = false;
+        setCanBeNegativeZero(false);
 
     // If rhs is a constant int != -1, likewise.
     if (rhs()->isConstant() && !rhs()->toConstant()->value().isInt32(-1))
-        canBeNegativeOverflow_ = false;
+        setCanBeNegativeZero(false);
 
     // If lhs is != 0, then negative zero check can be skipped.
     if (lhs()->isConstant() && !lhs()->toConstant()->value().isInt32(0))
-        canBeNegativeZero_ = false;
+        setCanBeNegativeZero(false);
 
     // If rhs is >= 0, likewise.
     if (rhs()->isConstant()) {
         const js::Value &val = rhs()->toConstant()->value();
         if (val.isInt32() && val.toInt32() >= 0)
-            canBeNegativeZero_ = false;
+            setCanBeNegativeZero(false);
     }
 }
 
 void
 MDiv::analyzeEdgeCasesBackward()
 {
-    if (canBeNegativeZero_)
-        canBeNegativeZero_ = NeedNegativeZeroCheck(this);
+    if (canBeNegativeZero() && !NeedNegativeZeroCheck(this))
+        setCanBeNegativeZero(false);
 }
 
 void
 MDiv::analyzeTruncateBackward()
 {
-    if (!isTruncated())
-        setTruncated(js::ion::EdgeCaseAnalysis::AllUsesTruncate(this));
+    if (!isTruncated() && js::ion::EdgeCaseAnalysis::AllUsesTruncate(this))
+        setTruncated(true);
 }
 
 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.
-    if (isTruncated())
-        setTruncated(ins->isTruncated());
+    setTruncated(isTruncated() && ins->isTruncated());
     return true;
 }
 
 static inline MDefinition *
 TryFold(MDefinition *original, MDefinition *replacement)
 {
     if (original->type() == replacement->type())
         return replacement;
@@ -834,50 +842,48 @@ MMod::foldsTo(bool useValueNumbers)
         return TryFold(this, lhs());
 
     return this;
 }
 
 void
 MAdd::analyzeTruncateBackward()
 {
-    if (!isTruncated())
-        setTruncated(js::ion::EdgeCaseAnalysis::AllUsesTruncate(this));
+    if (!isTruncated() && js::ion::EdgeCaseAnalysis::AllUsesTruncate(this))
+        setTruncated(true);
 }
 
 bool
 MAdd::updateForReplacement(MDefinition *ins_)
 {
     JS_ASSERT(ins_->isAdd());
     MAdd *ins = ins_->toAdd();
-    if (isTruncated())
-        setTruncated(ins->isTruncated());
+    setTruncated(isTruncated() && ins->isTruncated());
     return true;
 }
 
 bool
 MAdd::fallible()
 {
     return !isTruncated() && (!range() || !range()->isFinite());
 }
 
 void
 MSub::analyzeTruncateBackward()
 {
-    if (!isTruncated())
-        setTruncated(js::ion::EdgeCaseAnalysis::AllUsesTruncate(this));
+    if (!isTruncated() && js::ion::EdgeCaseAnalysis::AllUsesTruncate(this))
+        setTruncated(true);
 }
 
 bool
 MSub::updateForReplacement(MDefinition *ins_)
 {
     JS_ASSERT(ins_->isSub());
     MSub *ins = ins_->toSub();
-    if (isTruncated())
-        setTruncated(ins->isTruncated());
+    setTruncated(isTruncated() && ins->isTruncated());
     return true;
 }
 
 bool
 MSub::fallible()
 {
     return !isTruncated() && (!range() || !range()->isFinite());
 }
@@ -888,92 +894,86 @@ MMul::foldsTo(bool useValueNumbers)
     MDefinition *out = MBinaryArithInstruction::foldsTo(useValueNumbers);
     if (out != this)
         return out;
 
     if (specialization() != MIRType_Int32)
         return this;
 
     if (EqualValues(useValueNumbers, lhs(), rhs()))
-        canBeNegativeZero_ = false;
+        setCanBeNegativeZero(false);
 
     return this;
 }
 
 void
 MMul::analyzeEdgeCasesForward()
 {
     // Try to remove the check for negative zero
     // This only makes sense when using the integer multiplication
     if (specialization() != MIRType_Int32)
         return;
 
     // If lhs is > 0, no need for negative zero check.
     if (lhs()->isConstant()) {
         const js::Value &val = lhs()->toConstant()->value();
         if (val.isInt32() && val.toInt32() > 0)
-            canBeNegativeZero_ = false;
+            setCanBeNegativeZero(false);
     }
 
     // If rhs is > 0, likewise.
     if (rhs()->isConstant()) {
         const js::Value &val = rhs()->toConstant()->value();
         if (val.isInt32() && val.toInt32() > 0)
-            canBeNegativeZero_ = false;
+            setCanBeNegativeZero(false);
     }
 
 }
 
 void
 MMul::analyzeEdgeCasesBackward()
 {
-    if (canBeNegativeZero_)
-        canBeNegativeZero_ = NeedNegativeZeroCheck(this);
+    if (canBeNegativeZero() && !NeedNegativeZeroCheck(this))
+        setCanBeNegativeZero(false);
 }
 
 void
 MMul::analyzeTruncateBackward()
 {
-    if (!isPossibleTruncated())
-        setPossibleTruncated(js::ion::EdgeCaseAnalysis::AllUsesTruncate(this));
+    if (!isPossibleTruncated() && js::ion::EdgeCaseAnalysis::AllUsesTruncate(this))
+        setPossibleTruncated(true);
 }
 
 bool
 MMul::updateForReplacement(MDefinition *ins_)
 {
     JS_ASSERT(ins_->isMul());
     MMul *ins = ins_->toMul();
-    if (isPossibleTruncated())
-        setPossibleTruncated(ins->isPossibleTruncated());
+    // setPossibleTruncated can reset the canBenegativeZero check,
+    // therefore first query the state, before setting the new state.
+    bool truncated = isPossibleTruncated() && ins->isPossibleTruncated();
+    bool negativeZero = canBeNegativeZero() || ins->canBeNegativeZero();
+    setPossibleTruncated(truncated);
+    setCanBeNegativeZero(negativeZero);
     return true;
 }
 
 bool
 MMul::canOverflow()
 {
     if (implicitTruncate_)
         return false;
     return !range() || !range()->isFinite();
 }
 
-bool
-MMul::canBeNegativeZero()
-{
-    if (!range())
-        return canBeNegativeZero_;
-    if (range()->lower() > 0 || range()->upper() < 0)
-        return false;
-    return canBeNegativeZero_;
-}
-
 void
 MBinaryArithInstruction::infer(JSContext *cx, const TypeOracle::BinaryTypes &b)
 {
     // Retrieve type information of lhs and rhs
-    // Rhs is defaulted to int32 first, 
+    // Rhs is defaulted to int32 first,
     // because in some cases there is no rhs type information
     MIRType lhs = MIRTypeFromValueType(b.lhsTypes->getKnownTypeTag());
     MIRType rhs = MIRType_Int32;
 
     // Test if types coerces to doubles
     bool lhsCoerces = b.lhsTypes->knownNonStringPrimitive();
     bool rhsCoerces = true;
 
@@ -1279,17 +1279,18 @@ MToInt32::foldsTo(bool useValueNumbers)
     if (input->type() == MIRType_Int32)
         return input;
     return this;
 }
 
 void
 MToInt32::analyzeEdgeCasesBackward()
 {
-    canBeNegativeZero_ = NeedNegativeZeroCheck(this);
+    if (!NeedNegativeZeroCheck(this))
+        setCanBeNegativeZero(false);
 }
 
 MDefinition *
 MTruncateToInt32::foldsTo(bool useValueNumbers)
 {
     MDefinition *input = getOperand(0);
     if (input->type() == MIRType_Int32)
         return input;
--- a/js/src/ion/MIR.h
+++ b/js/src/ion/MIR.h
@@ -1754,16 +1754,19 @@ class MToInt32 : public MUnaryInstructio
     MDefinition *foldsTo(bool useValueNumbers);
 
     // this only has backwards information flow.
     void analyzeEdgeCasesBackward();
 
     bool canBeNegativeZero() {
         return canBeNegativeZero_;
     }
+    void setCanBeNegativeZero(bool negativeZero) {
+        canBeNegativeZero_ = negativeZero;
+    }
 
     bool congruentTo(MDefinition *const &ins) const {
         return congruentIfOperandsEqual(ins);
     }
 
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
@@ -2433,18 +2436,18 @@ class MAdd : public MBinaryArithInstruct
     static MAdd *New(MDefinition *left, MDefinition *right) {
         return new MAdd(left, right);
     }
     void analyzeTruncateBackward();
 
     bool isTruncated() const {
         return implicitTruncate_;
     }
-    void setTruncated(bool val) {
-        implicitTruncate_ = val;
+    void setTruncated(bool truncate) {
+        implicitTruncate_ = truncate;
     }
     bool updateForReplacement(MDefinition *ins);
     double getIdentity() {
         return 0;
     }
 
     bool fallible();
     void computeRange();
@@ -2465,18 +2468,18 @@ class MSub : public MBinaryArithInstruct
     static MSub *New(MDefinition *left, MDefinition *right) {
         return new MSub(left, right);
     }
 
     void analyzeTruncateBackward();
     bool isTruncated() const {
         return implicitTruncate_;
     }
-    void setTruncated(bool val) {
-        implicitTruncate_ = val;
+    void setTruncated(bool truncate) {
+        implicitTruncate_ = truncate;
     }
     bool updateForReplacement(MDefinition *ins);
 
     double getIdentity() {
         return 0;
     }
 
     bool fallible();
@@ -2523,32 +2526,45 @@ class MMul : public MBinaryArithInstruct
     void analyzeEdgeCasesBackward();
     void analyzeTruncateBackward();
 
     double getIdentity() {
         return 1;
     }
 
     bool canOverflow();
-    bool canBeNegativeZero();
+
+    bool canBeNegativeZero() {
+        return canBeNegativeZero_;
+    }
+    void setCanBeNegativeZero(bool negativeZero) {
+        canBeNegativeZero_ = negativeZero;
+    }
 
     bool updateForReplacement(MDefinition *ins);
 
     bool fallible() {
         return canBeNegativeZero_ || canOverflow();
     }
 
     void computeRange();
 
     bool isPossibleTruncated() const {
         return possibleTruncate_;
     }
 
     void setPossibleTruncated(bool truncate) {
         possibleTruncate_ = truncate;
+
+        // We can remove the negative zero check, because op if it is only used truncated.
+        // The "Possible" in the function name means that we are not sure,
+        // that "integer mul and disregarding overflow" == "double mul and ToInt32"
+        // Note: when removing truncated state, we have to add negative zero check again,
+        // because we are not sure if it was removed by this or other passes.
+        canBeNegativeZero_ = !truncate;
     }
 };
 
 class MDiv : public MBinaryArithInstruction
 {
     bool canBeNegativeZero_;
     bool canBeNegativeOverflow_;
     bool canBeDivideByZero_;
@@ -2584,23 +2600,26 @@ class MDiv : public MBinaryArithInstruct
     double getIdentity() {
         JS_NOT_REACHED("not used");
         return 1;
     }
 
     bool isTruncated() const {
         return implicitTruncate_;
     }
-    void setTruncated(bool val) {
-        implicitTruncate_ = val;
+    void setTruncated(bool truncate) {
+        implicitTruncate_ = truncate;
     }
 
     bool canBeNegativeZero() {
         return canBeNegativeZero_;
     }
+    void setCanBeNegativeZero(bool negativeZero) {
+        canBeNegativeZero_ = negativeZero;
+    }
 
     bool canBeNegativeOverflow() {
         return canBeNegativeOverflow_;
     }
 
     bool canBeDivideByZero() {
         return canBeDivideByZero_;
     }
--- a/js/src/ion/RangeAnalysis.cpp
+++ b/js/src/ion/RangeAnalysis.cpp
@@ -470,16 +470,37 @@ Range::precisionLossMul(const Range *lhs
         lower = -lower;
     if (upper < 0)
         upper = -upper;
 
     return lower > loss || upper > loss;
 }
 
 bool
+Range::negativeZeroMul(const Range *lhs, const Range *rhs)
+{
+    EnsureRange(&lhs);
+    EnsureRange(&rhs);
+
+    // Both values are positive
+    if (lhs->lower_ >= 0 && rhs->lower_ >= 0)
+        return false;
+
+    // Both values are negative (non zero)
+    if (lhs->upper_ < 0 && rhs->upper_ < 0)
+        return false;
+
+    // One operand is positive (non zero)
+    if (lhs->lower_ > 0 || rhs->lower_ > 0)
+        return false;
+
+    return true;
+}
+
+bool
 Range::update(const Range *other)
 {
     bool changed =
         lower_ != other->lower_ ||
         lower_infinite_ != other->lower_infinite_ ||
         upper_ != other->upper_ ||
         upper_infinite_ != other->upper_infinite_;
     if (changed) {
@@ -625,16 +646,18 @@ void
 MMul::computeRange()
 {
     if (specialization() != MIRType_Int32)
         return;
     Range *left = getOperand(0)->range();
     Range *right = getOperand(1)->range();
     if (isPossibleTruncated())
         implicitTruncate_ = !Range::precisionLossMul(left, right);
+    if (canBeNegativeZero())
+        canBeNegativeZero_ = Range::negativeZeroMul(left, right);
     setRange(Range::mul(left, right));
 }
 
 void
 MMod::computeRange()
 {
     if (specialization() != MIRType_Int32)
         return;
--- a/js/src/ion/RangeAnalysis.h
+++ b/js/src/ion/RangeAnalysis.h
@@ -178,16 +178,17 @@ class Range : public TempObject {
     static Range * add(const Range *lhs, const Range *rhs);
     static Range * sub(const Range *lhs, const Range *rhs);
     static Range * mul(const Range *lhs, const Range *rhs);
     static Range * and_(const Range *lhs, const Range *rhs);
     static Range * shl(const Range *lhs, int32 c);
     static Range * shr(const Range *lhs, int32 c);
 
     static bool precisionLossMul(const Range *lhs, const Range *rhs);
+    static bool negativeZeroMul(const Range *lhs, const Range *rhs);
 
     inline void makeLowerInfinite() {
         lower_infinite_ = true;
         lower_ = JSVAL_INT_MIN;
     }
     inline void makeUpperInfinite() {
         upper_infinite_ = true;
         upper_ = JSVAL_INT_MAX;