Try to coerce doubles to integers on tripped type barriers, bug 678687. r=dvander

js/src/jsinfer.cpp

@@ -4954,7 +4954,7 @@ MarkIteratorUnknownSlow(JSContext *cx)
 
 void
 TypeMonitorCallSlow(JSContext *cx, JSObject *callee,
-                    const CallArgs &args, bool constructing)
+                    CallArgs &args, bool constructing)
 {
     unsigned nargs = callee->getFunctionPrivate()->nargs;
     JSScript *script = callee->getFunctionPrivate()->script();
@@ -4972,8 +4972,10 @@ TypeMonitorCallSlow(JSContext *cx, JSObject *callee,
         TypeScript::SetArgument(cx, script, arg, args[arg]);
 
     /* Watch for fewer actuals than formals to the call. */
-    for (; arg < nargs; arg++)
-        TypeScript::SetArgument(cx, script, arg, UndefinedValue());
+    for (; arg < nargs; arg++) {
+        Value v = UndefinedValue();
+        TypeScript::SetArgument(cx, script, arg, v);
+    }
 }
 
 static inline bool
@@ -5091,8 +5093,10 @@ TypeDynamicResult(JSContext *cx, JSScript *script, jsbytecode *pc, Type type)
 }
 
 void
-TypeMonitorResult(JSContext *cx, JSScript *script, jsbytecode *pc, const js::Value &rval)
+TypeMonitorResult(JSContext *cx, JSScript *script, jsbytecode *pc, js::Value &rval)
 {
+    TryCoerceNumberToInt32(rval);
+
     UntrapOpcode untrap(cx, script, pc);
 
     /* Allow the non-TYPESET scenario to simplify stubs used in compound opcodes. */

js/src/jsinfer.h

@@ -1077,9 +1077,14 @@ class TypeScript
      * by type inference or where the pc has type barriers. For simplicity, we
      * always monitor JOF_TYPESET opcodes in the interpreter and stub calls,
      * and only look at barriers when generating JIT code for the script.
+     *
+     * 'val' is passed by reference, and must refer to the location of the op's
+     * result which will be subsequently used (i.e. not a temporary value).
+     * This call may change val by coercing a double-representable-as-an-int
+     * into an integer.
      */
     static inline void Monitor(JSContext *cx, JSScript *script, jsbytecode *pc,
-                               const js::Value &val);
+                               js::Value &val);
 
     /* Monitor an assignment at a SETELEM on a non-integer identifier. */
     static inline void MonitorAssign(JSContext *cx, JSScript *script, jsbytecode *pc,
@@ -1091,7 +1096,7 @@ class TypeScript
     static inline void SetLocal(JSContext *cx, JSScript *script, unsigned local, Type type);
     static inline void SetLocal(JSContext *cx, JSScript *script, unsigned local, const js::Value &value);
     static inline void SetArgument(JSContext *cx, JSScript *script, unsigned arg, Type type);
-    static inline void SetArgument(JSContext *cx, JSScript *script, unsigned arg, const js::Value &value);
+    static inline void SetArgument(JSContext *cx, JSScript *script, unsigned arg, js::Value &value);
 
     static void Sweep(JSContext *cx, JSScript *script);
     inline void trace(JSTracer *trc);

js/src/jsinferinlines.h

@@ -310,14 +310,15 @@ MarkIteratorUnknown(JSContext *cx)
 }
 
 /*
- * Monitor a javascript call, either on entry to the interpreter or made
- * from within the interpreter.
+ * Monitor a javascript call, either on entry to the interpreter or made from
+ * within the interpreter. As with TypeScript::Monitor, this may coerce double
+ * arguments to the call into integers.
  */
 inline void
-TypeMonitorCall(JSContext *cx, const js::CallArgs &args, bool constructing)
+TypeMonitorCall(JSContext *cx, js::CallArgs &args, bool constructing)
 {
     extern void TypeMonitorCallSlow(JSContext *cx, JSObject *callee,
-                                    const CallArgs &args, bool constructing);
+                                    CallArgs &args, bool constructing);
 
     JSObject *callee = &args.callee();
     if (callee->isFunction()) {
@@ -444,7 +445,7 @@ FixObjectType(JSContext *cx, JSObject *obj)
 }
 
 /* Interface helpers for JSScript */
-extern void TypeMonitorResult(JSContext *cx, JSScript *script, jsbytecode *pc, const js::Value &rval);
+extern void TypeMonitorResult(JSContext *cx, JSScript *script, jsbytecode *pc, js::Value &rval);
 extern void TypeDynamicResult(JSContext *cx, JSScript *script, jsbytecode *pc, js::types::Type type);
 
 inline bool
@@ -559,7 +560,7 @@ TypeScript::InitObject(JSContext *cx, JSScript *script, const jsbytecode *pc, JS
 }
 
 /* static */ inline void
-TypeScript::Monitor(JSContext *cx, JSScript *script, jsbytecode *pc, const js::Value &rval)
+TypeScript::Monitor(JSContext *cx, JSScript *script, jsbytecode *pc, js::Value &rval)
 {
     if (cx->typeInferenceEnabled())
         TypeMonitorResult(cx, script, pc, rval);
@@ -677,9 +678,10 @@ TypeScript::SetArgument(JSContext *cx, JSScript *script, unsigned arg, Type type
 }
 
 /* static */ inline void
-TypeScript::SetArgument(JSContext *cx, JSScript *script, unsigned arg, const js::Value &value)
+TypeScript::SetArgument(JSContext *cx, JSScript *script, unsigned arg, js::Value &value)
 {
     if (cx->typeInferenceEnabled()) {
+        TryCoerceNumberToInt32(value);
         Type type = GetValueType(cx, value);
         SetArgument(cx, script, arg, type);
     }

js/src/jsinterp.cpp

@@ -3488,10 +3488,10 @@ BEGIN_CASE(JSOP_LENGTH)
         }
     } while (0);
 
-    TypeScript::Monitor(cx, script, regs.pc, rval);
-
     regs.sp[-1] = rval;
     assertSameCompartment(cx, regs.sp[-1]);
+
+    TypeScript::Monitor(cx, script, regs.pc, regs.sp[-1]);
 }
 END_CASE(JSOP_GETPROP)
 
@@ -3583,7 +3583,7 @@ BEGIN_CASE(JSOP_CALLPROP)
             goto error;
     }
 #endif
-    TypeScript::Monitor(cx, script, regs.pc, rval);
+    TypeScript::Monitor(cx, script, regs.pc, regs.sp[-2]);
 }
 END_CASE(JSOP_CALLPROP)
 
@@ -4089,7 +4089,7 @@ BEGIN_CASE(JSOP_CALLNAME)
     }
 
     PUSH_COPY(rval);
-    TypeScript::Monitor(cx, script, regs.pc, rval);
+    TypeScript::Monitor(cx, script, regs.pc, regs.sp[-1]);
 
     /* obj must be on the scope chain, thus not a function. */
     if (op == JSOP_CALLNAME || op == JSOP_CALLGNAME)

js/src/jsnum.h

@@ -658,6 +658,17 @@ ToInteger(JSContext *cx, const js::Value &v, jsdouble *dp)
     return true;
 }
 
+/* If v is a double value which fits in an int32, coerce it to that int32. */
+static inline void
+TryCoerceNumberToInt32(Value &v)
+{
+    if (v.isDouble()) {
+        int32_t res;
+        if (JSDOUBLE_IS_INT32(v.toDouble(), &res))
+            v.setInt32(res);
+    }
+}
+
 } /* namespace js */
 
 #endif /* jsnum_h___ */

js/src/methodjit/BaseAssembler.h

@@ -230,6 +230,17 @@ static const JSC::MacroAssembler::RegisterID JSParamReg_Argc  = JSC::SparcRegist
     }
 #endif
 
+    void moveDoubleRegisters(RegisterID data, RegisterID type, Address address, FPRegisterID fpreg)
+    {
+#ifdef JS_CPU_X86
+        fastLoadDouble(data, type, fpreg);
+#else
+        /* Store the components, then read it back out as a double. */
+        storeValueFromComponents(type, data, address);
+        loadDouble(address, fpreg);
+#endif
+    }
+
     /*
      * Move a register pair which may indicate either an int32 or double into fpreg,
      * converting to double in the int32 case.
@@ -1164,6 +1175,29 @@ static const JSC::MacroAssembler::RegisterID JSParamReg_Argc  = JSC::SparcRegist
         } else if (types->hasType(types::Type::Int32Type())) {
             if (!matches.append(testInt32(Assembler::Equal, address)))
                 return false;
+
+            /* Generate a path to try coercing doubles to integers in place. */
+            Jump notDouble = testDouble(Assembler::NotEqual, address);
+
+            Registers tempRegs(Registers::AvailRegs);
+            RegisterID T1 = tempRegs.takeAnyReg().reg();
+
+            Registers tempFPRegs(Registers::TempFPRegs);
+            FPRegisterID FP1 = tempFPRegs.takeAnyReg().fpreg();
+            FPRegisterID FP2 = tempFPRegs.takeAnyReg().fpreg();
+
+            loadDouble(address, FP1);
+
+            JumpList isDouble;
+            branchConvertDoubleToInt32(FP1, T1, isDouble, FP2);
+
+            storeValueFromComponents(ImmType(JSVAL_TYPE_INT32), T1, address);
+
+            if (!matches.append(jump()))
+                return false;
+
+            isDouble.linkTo(label(), this);
+            notDouble.linkTo(label(), this);
         }
 
         if (types->hasType(types::Type::UndefinedType())) {

js/src/methodjit/Compiler.cpp

@@ -7369,39 +7369,6 @@ mjit::Compiler::pushAddressMaybeBarrier(Address address, JSValueType type, bool
     return testBarrier(typeReg, dataReg, testUndefined);
 }
 
-MaybeJump
-mjit::Compiler::trySingleTypeTest(types::TypeSet *types, RegisterID typeReg)
-{
-    /*
-     * If a type set we have a barrier on is monomorphic, generate a single
-     * jump taken if a type register has a match. This doesn't handle type sets
-     * containing objects, as these require two jumps regardless (test for
-     * object, then test the type of the object).
-     */
-    MaybeJump res;
-
-    switch (types->getKnownTypeTag(cx)) {
-      case JSVAL_TYPE_INT32:
-        res.setJump(masm.testInt32(Assembler::NotEqual, typeReg));
-        return res;
-
-      case JSVAL_TYPE_DOUBLE:
-        res.setJump(masm.testNumber(Assembler::NotEqual, typeReg));
-        return res;
-
-      case JSVAL_TYPE_BOOLEAN:
-        res.setJump(masm.testBoolean(Assembler::NotEqual, typeReg));
-        return res;
-
-      case JSVAL_TYPE_STRING:
-        res.setJump(masm.testString(Assembler::NotEqual, typeReg));
-        return res;
-
-      default:
-        return res;
-    }
-}
-
 JSC::MacroAssembler::Jump
 mjit::Compiler::addTypeTest(types::TypeSet *types, RegisterID typeReg, RegisterID dataReg)
 {
@@ -7413,11 +7380,32 @@ mjit::Compiler::addTypeTest(types::TypeSet *types, RegisterID typeReg, RegisterI
 
     Vector<Jump> matches(CompilerAllocPolicy(cx, *this));
 
-    if (types->hasType(types::Type::Int32Type()))
+    if (types->hasType(types::Type::DoubleType())) {
+        matches.append(masm.testNumber(Assembler::Equal, typeReg));
+    } else if (types->hasType(types::Type::Int32Type())) {
         matches.append(masm.testInt32(Assembler::Equal, typeReg));
 
-    if (types->hasType(types::Type::DoubleType()))
-        matches.append(masm.testDouble(Assembler::Equal, typeReg));
+        /* Generate a path to try coercing doubles to integers in place. */
+        Jump notDouble = masm.testDouble(Assembler::NotEqual, typeReg);
+
+        FPRegisterID fpTemp = frame.getScratchFPReg();
+        masm.moveDoubleRegisters(dataReg, typeReg, frame.addressOfTop(), fpTemp);
+
+        JumpList isDouble;
+        masm.branchConvertDoubleToInt32(fpTemp, dataReg, isDouble, Registers::FPConversionTemp);
+        masm.move(ImmType(JSVAL_TYPE_INT32), typeReg);
+
+        frame.restoreScratchFPReg(fpTemp);
+
+        matches.append(masm.jump());
+
+        isDouble.linkTo(masm.label(), &masm);
+
+        masm.breakDouble(fpTemp, typeReg, dataReg);
+        frame.restoreScratchFPReg(fpTemp);
+
+        notDouble.linkTo(masm.label(), &masm);
+    }
 
     if (types->hasType(types::Type::UndefinedType()))
         matches.append(masm.testUndefined(Assembler::Equal, typeReg));
@@ -7511,9 +7499,7 @@ mjit::Compiler::testBarrier(RegisterID typeReg, RegisterID dataReg,
     /* Cannot have type barriers when the result of the operation is already unknown. */
     JS_ASSERT(!types->unknown());
 
-    state.jump = trySingleTypeTest(types, typeReg);
-    if (!state.jump.isSet())
-        state.jump.setJump(addTypeTest(types, typeReg, dataReg));
+    state.jump.setJump(addTypeTest(types, typeReg, dataReg));
 
     return state;
 }

js/src/methodjit/Compiler.h

@@ -556,7 +556,6 @@ private:
         RegisterID dataReg;
     };
 
-    MaybeJump trySingleTypeTest(types::TypeSet *types, RegisterID typeReg);
     Jump addTypeTest(types::TypeSet *types, RegisterID typeReg, RegisterID dataReg);
     BarrierState pushAddressMaybeBarrier(Address address, JSValueType type, bool reuseBase,
                                          bool testUndefined = false);

js/src/methodjit/FastOps.cpp

@@ -1974,7 +1974,6 @@ mjit::Compiler::jsop_getelem_typed(int atype)
 
     frame.popn(2);
 
-    BarrierState barrier;
     if (dataReg.isFPReg()) {
         frame.pushDouble(dataReg.fpreg());
     } else if (typeReg.isSet()) {
@@ -1985,8 +1984,6 @@ mjit::Compiler::jsop_getelem_typed(int atype)
     }
     stubcc.rejoin(Changes(2));
 
-    finishBarrier(barrier, REJOIN_FALLTHROUGH, 0);
-
     return true;
 }
 #endif /* JS_METHODJIT_TYPED_ARRAY */
@@ -2010,7 +2007,12 @@ mjit::Compiler::jsop_getelem(bool isCall)
     if (cx->typeInferenceEnabled() && id->mightBeType(JSVAL_TYPE_INT32) && !isCall) {
         types::TypeSet *types = analysis->poppedTypes(PC, 1);
         if (types->isLazyArguments(cx) && !outerScript->analysis()->modifiesArguments()) {
-            // Inline arguments path.
+            // Inline arguments path. This path can access non-canonical args
+            // if fun->nargs != 0, so we require that the script never has any
+            // of its arguments directly modified. Note that the canonical and
+            // formal arg may not be the exact same value if we coerced a
+            // double actual into an int32 formal from a type barrier at entry,
+            // but this is ok as the underlying number is the same.
             jsop_getelem_args();
             return true;
         }

js/src/methodjit/FrameState.cpp

@@ -195,6 +195,36 @@ FrameState::takeReg(AnyRegisterID reg)
     }
 }
 
+JSC::MacroAssembler::FPRegisterID
+FrameState::getScratchFPReg()
+{
+    if (freeRegs.hasRegInMask(Registers::TempFPRegs)) {
+        FPRegisterID reg = freeRegs.takeAnyReg(Registers::TempFPRegs).fpreg();
+        freeRegs.putReg(reg);
+        return reg;
+    }
+
+    Registers regs(Registers::TempFPRegs);
+    FPRegisterID reg;
+
+    do {
+        reg = regs.takeAnyReg().fpreg();
+    } while (!regstate(reg).fe());
+
+    masm.storeDouble(reg, addressOf(regstate(reg).fe()));
+
+    return reg;
+}
+
+void
+FrameState::restoreScratchFPReg(FPRegisterID reg)
+{
+    if (freeRegs.hasReg(reg))
+        return;
+
+    masm.loadDouble(addressOf(regstate(reg).fe()), reg);
+}
+
 #ifdef DEBUG
 const char *
 FrameState::entryName(const FrameEntry *fe) const

js/src/methodjit/FrameState.h

@@ -613,6 +613,14 @@ class FrameState
      */
     void takeReg(AnyRegisterID reg);
 
+    /*
+     * Gets an FP register which the compiler does not own but can freely write
+     * over. Can only be used in the inline path, and must be matched with a
+     * restoreScratchFPReg. (Emits sync/restore code if the reg is in use).
+     */
+    FPRegisterID getScratchFPReg();
+    void restoreScratchFPReg(FPRegisterID reg);
+
     /*
      * Returns a FrameEntry * for a slot on the operation stack.
      */

js/src/methodjit/StubCalls.cpp

@@ -2367,7 +2367,7 @@ stubs::TypeBarrierHelper(VMFrame &f, uint32 which)
 void JS_FASTCALL
 stubs::StubTypeHelper(VMFrame &f, int32 which)
 {
-    const Value &result = f.regs.sp[which];
+    Value &result = f.regs.sp[which];
 
     if (f.script()->hasAnalysis() && f.script()->analysis()->ranInference()) {
         AutoEnterTypeInference enter(f.cx);