Bug 865431 - Restrict Compare_Double and related comparisons to inputs where they will produce the correct result, r=jandem.
authorBrian Hackett <bhackett1024@gmail.com>
Mon, 29 Apr 2013 13:10:00 -0600
changeset 141214 909360229cff08817fa3b9ffeac0c0c962aef502
parent 141213 db6188d1abe69a2accba25fd1bb72e755a46cb0a
child 141215 be2a35cd573964898f7f7227300eb16dfa98be72
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs865431
milestone23.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 865431 - Restrict Compare_Double and related comparisons to inputs where they will produce the correct result, r=jandem.
js/src/ion/BaselineIC.h
js/src/ion/BaselineInspector.cpp
js/src/ion/BaselineInspector.h
js/src/ion/CodeGenerator.cpp
js/src/ion/LIR-Common.h
js/src/ion/Lowering.cpp
js/src/ion/MIR.cpp
js/src/ion/MIR.h
js/src/ion/TypePolicy.cpp
--- a/js/src/ion/BaselineIC.h
+++ b/js/src/ion/BaselineIC.h
@@ -1913,25 +1913,31 @@ class ICCompare_Double : public ICStub
         }
     };
 };
 
 class ICCompare_NumberWithUndefined : public ICStub
 {
     friend class ICStubSpace;
 
-    ICCompare_NumberWithUndefined(IonCode *stubCode)
+    ICCompare_NumberWithUndefined(IonCode *stubCode, bool lhsIsUndefined)
       : ICStub(ICStub::Compare_NumberWithUndefined, stubCode)
-    {}
+    {
+        extra_ = lhsIsUndefined;
+    }
 
   public:
-    static inline ICCompare_NumberWithUndefined *New(ICStubSpace *space, IonCode *code) {
+    static inline ICCompare_NumberWithUndefined *New(ICStubSpace *space, IonCode *code, bool lhsIsUndefined) {
         if (!code)
             return NULL;
-        return space->allocate<ICCompare_NumberWithUndefined>(code);
+        return space->allocate<ICCompare_NumberWithUndefined>(code, lhsIsUndefined);
+    }
+
+    bool lhsIsUndefined() {
+        return extra_;
     }
 
     class Compiler : public ICMultiStubCompiler {
       protected:
         bool generateStubCode(MacroAssembler &masm);
 
         bool lhsIsUndefined;
 
@@ -1943,17 +1949,17 @@ class ICCompare_NumberWithUndefined : pu
 
         virtual int32_t getKey() const {
             return static_cast<int32_t>(kind)
                  | (static_cast<int32_t>(op) << 16)
                  | (static_cast<int32_t>(lhsIsUndefined) << 24);
         }
 
         ICStub *getStub(ICStubSpace *space) {
-            return ICCompare_NumberWithUndefined::New(space, getStubCode());
+            return ICCompare_NumberWithUndefined::New(space, getStubCode(), lhsIsUndefined);
         }
     };
 };
 
 class ICCompare_String : public ICStub
 {
     friend class ICStubSpace;
 
@@ -2075,17 +2081,17 @@ class ICCompare_ObjectWithUndefined : pu
         virtual int32_t getKey() const {
             return static_cast<int32_t>(kind)
                  | (static_cast<int32_t>(op) << 16)
                  | (static_cast<int32_t>(lhsIsUndefined) << 24)
                  | (static_cast<int32_t>(compareWithNull) << 25);
         }
 
         ICStub *getStub(ICStubSpace *space) {
-            return ICCompare_NumberWithUndefined::New(space, getStubCode());
+            return ICCompare_ObjectWithUndefined::New(space, getStubCode());
         }
     };
 };
 
 class ICCompare_Int32WithBoolean : public ICStub
 {
     friend class ICStubSpace;
 
--- a/js/src/ion/BaselineInspector.cpp
+++ b/js/src/ion/BaselineInspector.cpp
@@ -53,62 +53,64 @@ BaselineInspector::maybeMonomorphicShape
         if (next->toSetProp_Fallback()->hadUnoptimizableAccess())
             return NULL;
         return stub->toSetProp_Native()->shape();
     }
 
     return NULL;
 }
 
-ICStub::Kind
-BaselineInspector::monomorphicStubKind(jsbytecode *pc)
+ICStub *
+BaselineInspector::monomorphicStub(jsbytecode *pc)
 {
     if (!hasBaselineScript())
-        return ICStub::INVALID;
+        return NULL;
 
     const ICEntry &entry = icEntryFromPC(pc);
 
     ICStub *stub = entry.firstStub();
     ICStub *next = stub->next();
 
     if (!next || !next->isFallback())
-        return ICStub::INVALID;
+        return NULL;
 
-    return stub->kind();
+    return stub;
 }
 
 bool
-BaselineInspector::dimorphicStubKind(jsbytecode *pc, ICStub::Kind *pfirst, ICStub::Kind *psecond)
+BaselineInspector::dimorphicStub(jsbytecode *pc, ICStub **pfirst, ICStub **psecond)
 {
     if (!hasBaselineScript())
         return false;
 
     const ICEntry &entry = icEntryFromPC(pc);
 
     ICStub *stub = entry.firstStub();
     ICStub *next = stub->next();
     ICStub *after = next ? next->next() : NULL;
 
     if (!after || !after->isFallback())
         return false;
 
-    *pfirst = stub->kind();
-    *psecond = next->kind();
+    *pfirst = stub;
+    *psecond = next;
     return true;
 }
 
 MIRType
 BaselineInspector::expectedResultType(jsbytecode *pc)
 {
     // Look at the IC entries for this op to guess what type it will produce,
     // returning MIRType_None otherwise.
 
-    ICStub::Kind kind = monomorphicStubKind(pc);
+    ICStub *stub = monomorphicStub(pc);
+    if (!stub)
+        return MIRType_None;
 
-    switch (kind) {
+    switch (stub->kind()) {
       case ICStub::BinaryArith_Int32:
       case ICStub::BinaryArith_BooleanWithInt32:
       case ICStub::UnaryArith_Int32:
         return MIRType_Int32;
       case ICStub::BinaryArith_Double:
       case ICStub::BinaryArith_DoubleWithInt32:
       case ICStub::UnaryArith_Double:
         return MIRType_Double;
@@ -134,45 +136,52 @@ static bool
 CanUseInt32Compare(ICStub::Kind kind)
 {
     return kind == ICStub::Compare_Int32 || kind == ICStub::Compare_Int32WithBoolean;
 }
 
 MCompare::CompareType
 BaselineInspector::expectedCompareType(jsbytecode *pc)
 {
-    ICStub::Kind kind = monomorphicStubKind(pc);
+    ICStub *first = monomorphicStub(pc), *second = NULL;
+    if (!first && !dimorphicStub(pc, &first, &second))
+        return MCompare::Compare_Unknown;
 
-    if (CanUseInt32Compare(kind))
+    if (CanUseInt32Compare(first->kind()) && (!second || CanUseInt32Compare(second->kind())))
         return MCompare::Compare_Int32;
-    if (CanUseDoubleCompare(kind))
-        return MCompare::Compare_Double;
 
-    ICStub::Kind first, second;
-    if (dimorphicStubKind(pc, &first, &second)) {
-        if (CanUseInt32Compare(first) && CanUseInt32Compare(second))
-            return MCompare::Compare_Int32;
-        if (CanUseDoubleCompare(first) && CanUseDoubleCompare(second))
-            return MCompare::Compare_Double;
+    if (CanUseDoubleCompare(first->kind()) && (!second || CanUseDoubleCompare(second->kind()))) {
+        ICCompare_NumberWithUndefined *coerce =
+            first->isCompare_NumberWithUndefined()
+            ? first->toCompare_NumberWithUndefined()
+            : (second && second->isCompare_NumberWithUndefined())
+              ? second->toCompare_NumberWithUndefined()
+              : NULL;
+        if (coerce) {
+            return coerce->lhsIsUndefined()
+                   ? MCompare::Compare_DoubleMaybeCoerceLHS
+                   : MCompare::Compare_DoubleMaybeCoerceRHS;
+        }
+        return MCompare::Compare_Double;
     }
 
     return MCompare::Compare_Unknown;
 }
 
 static bool
-TryToSpecializeBinaryArithOp(ICStub::Kind *kinds,
-                             uint32_t nkinds,
+TryToSpecializeBinaryArithOp(ICStub **stubs,
+                             uint32_t nstubs,
                              MIRType *result)
 {
     bool sawInt32 = false;
     bool sawDouble = false;
     bool sawOther = false;
 
-    for (uint32_t i = 0; i < nkinds; i++) {
-        switch (kinds[i]) {
+    for (uint32_t i = 0; i < nstubs; i++) {
+        switch (stubs[i]->kind()) {
           case ICStub::BinaryArith_Int32:
             sawInt32 = true;
             break;
           case ICStub::BinaryArith_BooleanWithInt32:
             sawInt32 = true;
             break;
           case ICStub::BinaryArith_Double:
             sawDouble = true;
@@ -198,22 +207,24 @@ TryToSpecializeBinaryArithOp(ICStub::Kin
     *result = MIRType_Int32;
     return true;
 }
 
 MIRType
 BaselineInspector::expectedBinaryArithSpecialization(jsbytecode *pc)
 {
     MIRType result;
-    ICStub::Kind kinds[2];
+    ICStub *stubs[2];
 
-    kinds[0] = monomorphicStubKind(pc);
-    if (TryToSpecializeBinaryArithOp(kinds, 1, &result))
-        return result;
+    stubs[0] = monomorphicStub(pc);
+    if (stubs[0]) {
+        if (TryToSpecializeBinaryArithOp(stubs, 1, &result))
+            return result;
+    }
 
-    if (dimorphicStubKind(pc, &kinds[0], &kinds[1])) {
-        if (TryToSpecializeBinaryArithOp(kinds, 2, &result))
+    if (dimorphicStub(pc, &stubs[0], &stubs[1])) {
+        if (TryToSpecializeBinaryArithOp(stubs, 2, &result))
             return result;
     }
 
     return MIRType_None;
 }
 
--- a/js/src/ion/BaselineInspector.h
+++ b/js/src/ion/BaselineInspector.h
@@ -84,18 +84,18 @@ class BaselineInspector
         ICEntry *ent = NULL;
         if (hasBaselineScript()) {
             ent = &icEntryFromPC(pc);
             JS_ASSERT(ent->fallbackStub()->kind() == expectedFallbackKind);
         }
         return ICInspectorType(this, pc, ent);
     }
 
-    ICStub::Kind monomorphicStubKind(jsbytecode *pc);
-    bool dimorphicStubKind(jsbytecode *pc, ICStub::Kind *pfirst, ICStub::Kind *psecond);
+    ICStub *monomorphicStub(jsbytecode *pc);
+    bool dimorphicStub(jsbytecode *pc, ICStub **pfirst, ICStub **psecond);
 
   public:
     RawShape maybeMonomorphicShapeForPropertyOp(jsbytecode *pc);
 
     SetElemICInspector setElemICInspector(jsbytecode *pc) {
         return makeICInspector<SetElemICInspector>(pc, ICStub::SetElem_Fallback);
     }
 
--- a/js/src/ion/CodeGenerator.cpp
+++ b/js/src/ion/CodeGenerator.cpp
@@ -203,41 +203,59 @@ CodeGenerator::visitValueToInt32(LValueT
     return true;
 }
 
 static const double DoubleZero = 0.0;
 
 bool
 CodeGenerator::visitValueToDouble(LValueToDouble *lir)
 {
+    MToDouble *mir = lir->mir();
     ValueOperand operand = ToValue(lir, LValueToDouble::Input);
     FloatRegister output = ToFloatRegister(lir->output());
 
     Register tag = masm.splitTagForTest(operand);
 
-    Label isDouble, isInt32, isBool, isNull, done;
-    // Type-check switch.
+    Label isDouble, isInt32, isBool, isNull, isUndefined, done;
+    bool hasBoolean = false, hasNull = false, hasUndefined = false;
+
     masm.branchTestDouble(Assembler::Equal, tag, &isDouble);
     masm.branchTestInt32(Assembler::Equal, tag, &isInt32);
-    masm.branchTestBoolean(Assembler::Equal, tag, &isBool);
-    masm.branchTestNull(Assembler::Equal, tag, &isNull);
-
-    Assembler::Condition cond = masm.testUndefined(Assembler::NotEqual, tag);
-    if (!bailoutIf(cond, lir->snapshot()))
+
+    if (mir->conversion() != MToDouble::NumbersOnly) {
+        masm.branchTestBoolean(Assembler::Equal, tag, &isBool);
+        masm.branchTestUndefined(Assembler::Equal, tag, &isUndefined);
+        hasBoolean = true;
+        hasUndefined = true;
+        if (mir->conversion() != MToDouble::NonNullNonStringPrimitives) {
+            masm.branchTestNull(Assembler::Equal, tag, &isNull);
+            hasNull = true;
+        }
+    }
+
+    if (!bailout(lir->snapshot()))
         return false;
-    masm.loadStaticDouble(&js_NaN, output);
-    masm.jump(&done);
-
-    masm.bind(&isNull);
-    masm.loadStaticDouble(&DoubleZero, output);
-    masm.jump(&done);
-
-    masm.bind(&isBool);
-    masm.boolValueToDouble(operand, output);
-    masm.jump(&done);
+
+    if (hasNull) {
+        masm.bind(&isNull);
+        masm.loadStaticDouble(&DoubleZero, output);
+        masm.jump(&done);
+    }
+
+    if (hasUndefined) {
+        masm.bind(&isUndefined);
+        masm.loadStaticDouble(&js_NaN, output);
+        masm.jump(&done);
+    }
+
+    if (hasBoolean) {
+        masm.bind(&isBool);
+        masm.boolValueToDouble(operand, output);
+        masm.jump(&done);
+    }
 
     masm.bind(&isInt32);
     masm.int32ValueToDouble(operand, output);
     masm.jump(&done);
 
     masm.bind(&isDouble);
     masm.unboxDouble(operand, output);
     masm.bind(&done);
--- a/js/src/ion/LIR-Common.h
+++ b/js/src/ion/LIR-Common.h
@@ -2329,16 +2329,20 @@ class LInt32ToDouble : public LInstructi
 };
 
 // Convert a value to a double.
 class LValueToDouble : public LInstructionHelper<1, BOX_PIECES, 0>
 {
   public:
     LIR_HEADER(ValueToDouble)
     static const size_t Input = 0;
+
+    MToDouble *mir() {
+        return mir_->toToDouble();
+    }
 };
 
 // Convert a value to an int32.
 //   Input: components of a Value
 //   Output: 32-bit integer
 //   Bailout: undefined, string, object, or non-int32 double
 //   Temps: one float register
 //
--- a/js/src/ion/Lowering.cpp
+++ b/js/src/ion/Lowering.cpp
@@ -830,18 +830,22 @@ LIRGenerator::visitCompare(MCompare *com
             rhs = useAnyOrConstant(right);
         } else {
             rhs = useRegister(right);
         }
         return define(new LCompare(op, lhs, rhs), comp);
     }
 
     // Compare doubles.
-    if (comp->compareType() == MCompare::Compare_Double)
+    if (comp->compareType() == MCompare::Compare_Double ||
+        comp->compareType() == MCompare::Compare_DoubleMaybeCoerceLHS ||
+        comp->compareType() == MCompare::Compare_DoubleMaybeCoerceRHS)
+    {
         return define(new LCompareD(useRegister(left), useRegister(right)), comp);
+    }
 
     // Compare values.
     if (comp->compareType() == MCompare::Compare_Value) {
         LCompareV *lir = new LCompareV();
         if (!useBoxAtStart(lir, LCompareV::LhsInput, left))
             return false;
         if (!useBoxAtStart(lir, LCompareV::RhsInput, right))
             return false;
@@ -1371,34 +1375,40 @@ LIRGenerator::visitOsrScopeChain(MOsrSco
     LOsrScopeChain *lir = new LOsrScopeChain(useRegister(object->entry()));
     return define(lir, object);
 }
 
 bool
 LIRGenerator::visitToDouble(MToDouble *convert)
 {
     MDefinition *opd = convert->input();
+    MToDouble::ConversionKind conversion = convert->conversion();
 
     switch (opd->type()) {
       case MIRType_Value:
       {
         LValueToDouble *lir = new LValueToDouble();
         if (!useBox(lir, LValueToDouble::Input, opd))
             return false;
         return assignSnapshot(lir) && define(lir, convert);
       }
 
       case MIRType_Null:
+        JS_ASSERT(conversion != MToDouble::NumbersOnly && conversion != MToDouble::NonNullNonStringPrimitives);
         return lowerConstantDouble(0, convert);
 
       case MIRType_Undefined:
+        JS_ASSERT(conversion != MToDouble::NumbersOnly);
         return lowerConstantDouble(js_NaN, convert);
 
+      case MIRType_Boolean:
+        JS_ASSERT(conversion != MToDouble::NumbersOnly);
+        /* FALLTHROUGH */
+
       case MIRType_Int32:
-      case MIRType_Boolean:
       {
         LInt32ToDouble *lir = new LInt32ToDouble(useRegister(opd));
         return define(lir, convert);
       }
 
       case MIRType_Double:
         return redefine(convert, opd);
 
--- a/js/src/ion/MIR.cpp
+++ b/js/src/ion/MIR.cpp
@@ -1388,16 +1388,18 @@ MCompare::inputType()
       case Compare_Null:
         return MIRType_Null;
       case Compare_Boolean:
         return MIRType_Boolean;
       case Compare_UInt32:
       case Compare_Int32:
         return MIRType_Int32;
       case Compare_Double:
+      case Compare_DoubleMaybeCoerceLHS:
+      case Compare_DoubleMaybeCoerceRHS:
         return MIRType_Double;
       case Compare_String:
       case Compare_StrictString:
         return MIRType_String;
       case Compare_Object:
         return MIRType_Object;
       case Compare_Unknown:
       case Compare_Value:
@@ -1442,21 +1444,22 @@ MCompare::infer(JSContext *cx, BaselineI
 
     // Numeric comparisons against a double coerce to double.
     if (IsNumberType(lhs) && IsNumberType(rhs)) {
         compareType_ = Compare_Double;
         return;
     }
 
     // Any comparison is allowed except strict eq.
-    if (!strictEq &&
-        ((lhs == MIRType_Double && SafelyCoercesToDouble(getOperand(1))) ||
-         (rhs == MIRType_Double && SafelyCoercesToDouble(getOperand(0)))))
-    {
-        compareType_ = Compare_Double;
+    if (!strictEq && lhs == MIRType_Double && SafelyCoercesToDouble(getOperand(1))) {
+        compareType_ = Compare_DoubleMaybeCoerceRHS;
+        return;
+    }
+    if (!strictEq && rhs == MIRType_Double && SafelyCoercesToDouble(getOperand(0))) {
+        compareType_ = Compare_DoubleMaybeCoerceLHS;
         return;
     }
 
     // Handle object comparison.
     if (!relationalEq && lhs == MIRType_Object && rhs == MIRType_Object) {
         compareType_ = Compare_Object;
         return;
     }
--- a/js/src/ion/MIR.h
+++ b/js/src/ion/MIR.h
@@ -1777,16 +1777,19 @@ class MCompare
         Compare_Int32,
 
         // Int32 compared as unsigneds
         Compare_UInt32,
 
         // Double compared to Double
         Compare_Double,
 
+        Compare_DoubleMaybeCoerceLHS,
+        Compare_DoubleMaybeCoerceRHS,
+
         // String compared to String
         Compare_String,
 
         // Undefined compared to String
         // Null      compared to String
         // Boolean   compared to String
         // Int32     compared to String
         // Double    compared to String
@@ -2332,41 +2335,58 @@ class MPassArg : public MUnaryInstructio
 };
 
 // Converts a primitive (either typed or untyped) to a double. If the input is
 // not primitive at runtime, a bailout occurs.
 class MToDouble
   : public MUnaryInstruction,
     public ToDoublePolicy
 {
-    MToDouble(MDefinition *def)
-      : MUnaryInstruction(def)
+  public:
+    // Types of values which can be converted.
+    enum ConversionKind {
+        NonStringPrimitives,
+        NonNullNonStringPrimitives,
+        NumbersOnly
+    };
+
+  private:
+    ConversionKind conversion_;
+
+    MToDouble(MDefinition *def, ConversionKind conversion = NonStringPrimitives)
+      : MUnaryInstruction(def), conversion_(conversion)
     {
         setResultType(MIRType_Double);
         setMovable();
     }
 
   public:
     INSTRUCTION_HEADER(ToDouble)
-    static MToDouble *New(MDefinition *def) {
-        return new MToDouble(def);
+    static MToDouble *New(MDefinition *def, ConversionKind conversion = NonStringPrimitives) {
+        return new MToDouble(def, conversion);
     }
     static MToDouble *NewAsmJS(MDefinition *def) {
         return new MToDouble(def);
     }
 
+    ConversionKind conversion() const {
+        return conversion_;
+    }
+
     TypePolicy *typePolicy() {
         return this;
     }
 
     MDefinition *foldsTo(bool useValueNumbers);
     MDefinition *input() const {
         return getOperand(0);
     }
     bool congruentTo(MDefinition *const &ins) const {
+        if (!ins->isToDouble() || ins->toToDouble()->conversion() != conversion())
+            return false;
         return congruentIfOperandsEqual(ins);
     }
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
 
     void computeRange();
     bool truncate();
--- a/js/src/ion/TypePolicy.cpp
+++ b/js/src/ion/TypePolicy.cpp
@@ -180,19 +180,25 @@ ComparePolicy::adjustInputs(MInstruction
         // See BinaryArithPolicy::adjustInputs for an explanation of the following
         if (in->type() == MIRType_Object || in->type() == MIRType_String ||
             in->type() == MIRType_Undefined)
         {
             in = boxAt(def, in);
         }
 
         switch (type) {
-          case MIRType_Double:
-            replace = MToDouble::New(in);
+          case MIRType_Double: {
+            MToDouble::ConversionKind convert = MToDouble::NumbersOnly;
+            if (compare->compareType() == MCompare::Compare_DoubleMaybeCoerceLHS && i == 0)
+                convert = MToDouble::NonNullNonStringPrimitives;
+            else if (compare->compareType() == MCompare::Compare_DoubleMaybeCoerceRHS && i == 1)
+                convert = MToDouble::NonNullNonStringPrimitives;
+            replace = MToDouble::New(in, convert);
             break;
+          }
           case MIRType_Int32:
             replace = MToInt32::New(in);
             break;
           case MIRType_Object:
             replace = MUnbox::New(in, MIRType_Object, MUnbox::Infallible);
             break;
           case MIRType_String:
             replace = MUnbox::New(in, MIRType_String, MUnbox::Infallible);