Issue #2308 & #1240 Follow-up - Replace JSOP_POS in ++/-- with JSOP_TONUMERIC. https://bugzilla.mozilla.org/show_bug.cgi?id=1519135

js/src/frontend/BytecodeEmitter.cpp

@@ -2057,7 +2057,7 @@ BytecodeEmitter::emitCallIncDec(UnaryNode* incDec)
     MOZ_ASSERT(call->isKind(PNK_CALL));
     if (!emitTree(call))                                // CALLRESULT
         return false;
-    if (!emit1(JSOP_POS))                               // N
+    if (!emit1(JSOP_TONUMERIC))                         // N
         return false;
 
     // The increment/decrement has no side effects, so proceed to throw for

js/src/frontend/ElemOpEmitter.cpp

@@ -234,7 +234,7 @@ ElemOpEmitter::emitIncDec()
     MOZ_ASSERT(state_ == State::Get);
 
     JSOp incOp = isInc() ? JSOP_INC : JSOP_DEC;
-    if (!bce_->emit1(JSOP_POS)) {                     // ... N
+    if (!bce_->emit1(JSOP_TONUMERIC)) {               // ... N
         return false;
     }
     if (isPostIncDec()) {

js/src/frontend/NameOpEmitter.cpp

@@ -343,7 +343,7 @@ NameOpEmitter::emitIncDec()
     if (!prepareForRhs()) {                           // ENV? V
         return false;
     }
-    if (!bce_->emit1(JSOP_POS)) {                     // ENV? N
+    if (!bce_->emit1(JSOP_TONUMERIC)) {               // ENV? N
         return false;
     }
     if (isPostIncDec()) {

js/src/frontend/PropOpEmitter.cpp

@@ -219,7 +219,7 @@ PropOpEmitter::emitIncDec(JSAtom* prop)
 
     JSOp incOp = isInc() ? JSOP_INC : JSOP_DEC;
 
-    if (!bce_->emit1(JSOP_POS)) {                     // ... N
+    if (!bce_->emit1(JSOP_TONUMERIC)) {               // ... N
         return false;
     }
     if (isPostIncDec()) {

js/src/jit/BaselineCompiler.cpp

@@ -1322,14 +1322,37 @@ BaselineCompiler::emit_JSOP_POS()
     // Keep top stack value in R0.
     frame.popRegsAndSync(1);
 
-    // Inline path for int32 and double.
+    // Inline path for int32 and double; otherwise call VM.
     Label done;
     masm.branchTestNumber(Assembler::Equal, R0, &done);
 
-    // Call IC.
+    prepareVMCall();
-    ICToNumber_Fallback::Compiler stubCompiler(cx);
+    pushArg(R0);
-    if (!emitOpIC(stubCompiler.getStub(&stubSpace_)))
+    if (!callVM(ToNumberInfo)) {
+      return false;
+    }
+
+    masm.bind(&done);
+    frame.push(R0);
+    return true;
+}
+
+
+bool
+BaselineCompiler::emit_JSOP_TONUMERIC()
+{
+    // Keep top stack value in R0.
+    frame.popRegsAndSync(1);
+
+    // Inline path for int32 and double; otherwise call VM.
+    Label done;
+    masm.branchTestNumber(Assembler::Equal, R0, &done);
+
+    prepareVMCall();
+    pushArg(R0);
+    if (!callVM(ToNumericInfo)) {
         return false;
+    }
 
     masm.bind(&done);
     frame.push(R0);

js/src/jit/BaselineCompiler.h

@@ -50,6 +50,7 @@ namespace jit {
     _(JSOP_OR)                 \
     _(JSOP_NOT)                \
     _(JSOP_POS)                \
+    _(JSOP_TONUMERIC)          \
     _(JSOP_LOOPHEAD)           \
     _(JSOP_LOOPENTRY)          \
     _(JSOP_VOID)               \

js/src/jit/BaselineIC.cpp

@@ -694,42 +694,6 @@ ICToBool_Object::Compiler::generateStubCode(MacroAssembler& masm)
     return true;
 }
 
-//
-// ToNumber_Fallback
-//
-
-static bool
-DoToNumberFallback(JSContext* cx, ICToNumber_Fallback* stub, HandleValue arg, MutableHandleValue ret)
-{
-    FallbackICSpew(cx, stub, "ToNumber");
-    ret.set(arg);
-    return ToNumber(cx, ret);
-}
-
-typedef bool (*DoToNumberFallbackFn)(JSContext*, ICToNumber_Fallback*, HandleValue, MutableHandleValue);
-static const VMFunction DoToNumberFallbackInfo =
-    FunctionInfo<DoToNumberFallbackFn>(DoToNumberFallback, "DoToNumberFallback", TailCall,
-                                       PopValues(1));
-
-bool
-ICToNumber_Fallback::Compiler::generateStubCode(MacroAssembler& masm)
-{
-    MOZ_ASSERT(engine_ == Engine::Baseline);
-    MOZ_ASSERT(R0 == JSReturnOperand);
-
-    // Restore the tail call register.
-    EmitRestoreTailCallReg(masm);
-
-    // Ensure stack is fully synced for the expression decompiler.
-    masm.pushValue(R0);
-
-    // Push arguments.
-    masm.pushValue(R0);
-    masm.push(ICStubReg);
-
-    return tailCallVM(DoToNumberFallbackInfo, masm);
-}
-
 //
 // GetElem_Fallback
 //

js/src/jit/BaselineIC.h

@@ -333,32 +333,6 @@ class ICToBool_Object : public ICStub
     };
 };
 
-// ToNumber
-//     JSOP_POS
-
-class ICToNumber_Fallback : public ICFallbackStub
-{
-    friend class ICStubSpace;
-
-    explicit ICToNumber_Fallback(JitCode* stubCode)
-      : ICFallbackStub(ICStub::ToNumber_Fallback, stubCode) {}
-
-  public:
-    // Compiler for this stub kind.
-    class Compiler : public ICStubCompiler {
-      protected:
-        MOZ_MUST_USE bool generateStubCode(MacroAssembler& masm);
-
-      public:
-        explicit Compiler(JSContext* cx)
-          : ICStubCompiler(cx, ICStub::ToNumber_Fallback, Engine::Baseline) {}
-
-        ICStub* getStub(ICStubSpace* space) {
-            return newStub<ICToNumber_Fallback>(space, getStubCode());
-        }
-    };
-};
-
 // GetElem
 //      JSOP_GETELEM
 

js/src/jit/BaselineICList.h

@@ -34,8 +34,6 @@ namespace jit {
     _(ToBool_Double)                             \
     _(ToBool_Object)                             \
                                                  \
-    _(ToNumber_Fallback)                         \
-                                                 \
     _(Call_Fallback)                             \
     _(Call_Scripted)                             \
     _(Call_AnyScripted)                          \

js/src/jit/BaselineInspector.cpp

@@ -408,6 +408,11 @@ BaselineInspector::expectedBinaryArithSpecialization(jsbytecode* pc)
     MIRType result;
     ICStub* stubs[2];
 
+    if (JSOp(*pc) == JSOP_POS) {
+      // +x expanding to x*1, but no corresponding IC.
+      return MIRType::None;
+    }
+
     const ICEntry& entry = icEntryFromPC(pc);
     ICStub* stub = entry.fallbackStub();
     if (stub->isBinaryArith_Fallback() &&

js/src/jit/CodeGenerator.cpp

@@ -3415,6 +3415,48 @@ CodeGenerator::visitLoadUnboxedExpando(LLoadUnboxedExpando* lir)
     masm.loadPtr(Address(obj, UnboxedPlainObject::offsetOfExpando()), result);
 }
 
+void
+CodeGenerator::visitToNumeric(LToNumeric* lir)
+{
+    ValueOperand operand = ToValue(lir, LToNumeric::Input);
+    ValueOperand output = ToOutValue(lir);
+    bool maybeInt32 = lir->mir()->mightBeType(MIRType::Int32);
+    bool maybeDouble = lir->mir()->mightBeType(MIRType::Double);
+    bool maybeNumber = maybeInt32 || maybeDouble;
+    bool maybeBigInt = lir->mir()->mightBeType(MIRType::BigInt);
+    int checks = int(maybeNumber) + int(maybeBigInt);
+
+    OutOfLineCode* ool = oolCallVM(ToNumericInfo, lir, ArgList(operand), StoreValueTo(output));
+
+    if (checks == 0) {
+        masm.jump(ool->entry());
+    } else {
+        Label done;
+        using Condition = Assembler::Condition;
+        constexpr Condition Equal = Assembler::Equal;
+        constexpr Condition NotEqual = Assembler::NotEqual;
+
+        if (maybeNumber) {
+            checks--;
+            Condition cond = checks ? Equal : NotEqual;
+            Label* target = checks ? &done : ool->entry();
+            masm.branchTestNumber(cond, operand, target);
+        }
+        if (maybeBigInt) {
+            checks--;
+            Condition cond = checks ? Equal : NotEqual;
+            Label* target = checks ? &done : ool->entry();
+            masm.branchTestBigInt(cond, operand, target);
+        }
+
+        MOZ_ASSERT(checks == 0);
+        masm.bind(&done);
+        masm.moveValue(operand, output);
+    }
+
+    masm.bind(ool->rejoin());
+}
+
 void
 CodeGenerator::visitTypeBarrierV(LTypeBarrierV* lir)
 {

js/src/jit/CodeGenerator.h

@@ -149,6 +149,7 @@ class CodeGenerator final : public CodeGeneratorSpecific
     void visitGuardReceiverPolymorphic(LGuardReceiverPolymorphic* lir);
     void visitGuardUnboxedExpando(LGuardUnboxedExpando* lir);
     void visitLoadUnboxedExpando(LLoadUnboxedExpando* lir);
+    void visitToNumeric(LToNumeric* lir);
     void visitTypeBarrierV(LTypeBarrierV* lir);
     void visitTypeBarrierO(LTypeBarrierO* lir);
     void visitMonitorTypes(LMonitorTypes* lir);

js/src/jit/IonBuilder.cpp

@@ -636,7 +636,7 @@ IonBuilder::analyzeNewLoopTypes(MBasicBlock* entry, jsbytecode* start, jsbytecod
 
         MPhi* phi = entry->getSlot(slot)->toPhi();
 
-        if (*last == JSOP_POS)
+        if (*last == JSOP_POS || *last == JSOP_TONUMERIC)
             last = earlier;
 
         if (CodeSpec[*last].format & JOF_TYPESET) {
@@ -1586,6 +1586,7 @@ IonBuilder::traverseBytecode()
                 break;
 
               case JSOP_POS:
+              case JSOP_TONUMERIC:
               case JSOP_TOID:
               case JSOP_TOSTRING:
                 // These ops may leave their input on the stack without setting
@@ -1739,6 +1740,9 @@ IonBuilder::inspectOpcode(JSOp op)
       case JSOP_POS:
         return jsop_pos();
 
+      case JSOP_TONUMERIC:
+        return jsop_tonumeric();
+
       case JSOP_NEG:
         return jsop_neg();
 
@@ -5312,6 +5316,44 @@ IonBuilder::unaryArithTrySpecializedOnBaselineInspector(bool* emitted, JSOp op,
     return true;
 }
 
+bool
+IonBuilder::jsop_tonumeric()
+{
+    MDefinition* peeked = current->peek(-1);
+
+    if (IsNumericType(peeked->type())) {
+        // Elide the ToNumeric as we already unboxed the value.
+        peeked->setImplicitlyUsedUnchecked();
+        return true;
+    }
+
+    LifoAlloc* lifoAlloc = alloc().lifoAlloc();
+    TemporaryTypeSet* types = lifoAlloc->new_<TemporaryTypeSet>();
+    if (!types) {
+        return false;
+    }
+
+    types->addType(TypeSet::Int32Type(), lifoAlloc);
+    types->addType(TypeSet::DoubleType(), lifoAlloc);
+    types->addType(TypeSet::BigIntType(), lifoAlloc);
+
+    if (peeked->type() == MIRType::Value && peeked->resultTypeSet() &&
+        peeked->resultTypeSet()->isSubset(types)) {
+        // Elide the ToNumeric because the arg is already a boxed numeric.
+        peeked->setImplicitlyUsedUnchecked();
+        return true;
+    }
+
+    // Otherwise, pop the value and add an MToNumeric.
+    MDefinition* popped = current->pop();
+    MToNumeric* ins = MToNumeric::New(alloc(), popped, types);
+    current->add(ins);
+    current->push(ins);
+
+    // toValue() is effectful, so add a resume point.
+    return resumeAfter(ins);
+}
+
 bool
 IonBuilder::jsop_inc_or_dec(JSOp op)
 {

js/src/jit/IonBuilder.h

@@ -703,6 +703,7 @@ class IonBuilder
     MOZ_MUST_USE bool jsop_pow();
     MOZ_MUST_USE bool jsop_pos();
     MOZ_MUST_USE bool jsop_neg();
+    MOZ_MUST_USE bool jsop_tonumeric();
     MOZ_MUST_USE bool jsop_inc_or_dec(JSOp op);
     MOZ_MUST_USE bool jsop_tostring();
     MOZ_MUST_USE bool jsop_setarg(uint32_t arg);

js/src/jit/IonTypes.h

@@ -644,6 +644,10 @@ IsNumberType(MIRType type)
            type == MIRType::Int64;
 }
 
+static inline bool IsNumericType(MIRType type) {
+    return IsNumberType(type) || type == MIRType::BigInt;
+}
+
 static inline bool
 IsTypeRepresentableAsDouble(MIRType type)
 {

js/src/jit/Lowering.cpp

@@ -2165,6 +2165,15 @@ LIRGenerator::visitToInt32(MToInt32* convert)
     }
 }
 
+void
+LIRGenerator::visitToNumeric(MToNumeric* ins)
+{
+    MOZ_ASSERT(ins->input()->type() == MIRType::Value);
+    LToNumeric* lir = new (alloc()) LToNumeric(useBoxAtStart(ins->input()));
+    defineBox(lir, ins);
+    assignSafepoint(lir, ins);
+}
+
 void
 LIRGenerator::visitTruncateToInt32(MTruncateToInt32* truncate)
 {

js/src/jit/Lowering.h

@@ -336,6 +336,7 @@ class LIRGenerator : public LIRGeneratorSpecific
     void visitDebugCheckSelfHosted(MDebugCheckSelfHosted* ins);
     void visitModuleMetadata(MModuleMetadata* ins);
     void visitDynamicImport(MDynamicImport* ins);
+    void visitToNumeric(MToNumeric* ins);
 };
 
 } // namespace jit

js/src/jit/MIR.cpp

@@ -4172,6 +4172,23 @@ MResumePoint::isRecoverableOperand(MUse* u) const
     return block()->info().isRecoverableOperand(indexOf(u));
 }
 
+MDefinition*
+MToNumeric::foldsTo(TempAllocator& alloc)
+{
+    MDefinition* input = getOperand(0);
+
+    if (input->isBox()) {
+        MDefinition* unboxed = input->getOperand(0);
+        if (IsNumericType(unboxed->type())) {
+            // If the argument is an MBox and we can see that it boxes a numeric
+            // value, ToNumeric can be elided.
+            return input;
+        }
+    }
+
+    return this;
+}
+
 MDefinition*
 MToInt32::foldsTo(TempAllocator& alloc)
 {

js/src/jit/MIR.h

@@ -5534,6 +5534,43 @@ class MInt64ToFloatingPoint
     }
 };
 
+// Takes a boxed Value and returns a Value containing either a Number or a
+// BigInt.  Usually this will be the value itself, but it may be an object that
+// has a @@toPrimitive, valueOf, or toString method.
+class MToNumeric : public MUnaryInstruction, public BoxInputsPolicy::Data
+{
+    MToNumeric(MDefinition* arg, TemporaryTypeSet* types)
+               : MUnaryInstruction(arg)
+    {
+        MOZ_ASSERT(!IsNumericType(arg->type()),
+                   "Unboxable definitions don't need ToNumeric");
+        setResultType(MIRType::Value);
+        // Although `types' is always Int32|Double|BigInt, we have to compute it in
+        // IonBuilder to know whether emitting an MToNumeric is needed, so we just
+        // pass it through as an argument instead of recomputing it here.
+        setResultTypeSet(types);
+        setGuard();
+        setMovable();
+    }
+
+  public:
+    INSTRUCTION_HEADER(ToNumeric)
+    TRIVIAL_NEW_WRAPPERS
+
+        static MToNumeric* New(TempAllocator& alloc, MDefinition* arg,
+                               TemporaryTypeSet* types) {
+        return new (alloc) MToNumeric(arg, types);
+    }
+
+    void computeRange(TempAllocator& alloc) override;
+    bool congruentTo(const MDefinition* ins) const override {
+        return congruentIfOperandsEqual(ins);
+    }
+    MDefinition* foldsTo(TempAllocator& alloc) override;
+
+    ALLOW_CLONE(MToNumeric)
+};
+
 // Converts a primitive (either typed or untyped) to an int32. If the input is
 // not primitive at runtime, a bailout occurs. If the input cannot be converted
 // to an int32 without loss (i.e. "5.5" or undefined) then a bailout occurs.

js/src/jit/MOpcodes.h

@@ -123,6 +123,7 @@ namespace jit {
     _(ToDouble)                                                             \
     _(ToFloat32)                                                            \
     _(ToInt32)                                                              \
+    _(ToNumeric)                                                            \
     _(TruncateToInt32)                                                      \
     _(WrapInt64ToInt32)                                                     \
     _(ExtendInt32ToInt64)                                                   \

js/src/jit/RangeAnalysis.cpp

@@ -1741,6 +1741,12 @@ MTruncateToInt32::computeRange(TempAllocator& alloc)
     setRange(output);
 }
 
+void
+MToNumeric::computeRange(TempAllocator& alloc)
+{
+    setRange(new (alloc) Range(getOperand(0)));
+}
+
 void
 MToInt32::computeRange(TempAllocator& alloc)
 {

js/src/jit/TypePolicy.cpp

@@ -1186,6 +1186,7 @@ FilterTypeSetPolicy::adjustInputs(TempAllocator& alloc, MInstruction* ins)
 
 // Lists of all TypePolicy specializations which are used by MIR Instructions.
 #define TYPE_POLICY_LIST(_)                     \
+    _(AllDoublePolicy)                          \
     _(ArithPolicy)                              \
     _(BitwisePolicy)                            \
     _(BoxInputsPolicy)                          \
@@ -1204,7 +1205,6 @@ FilterTypeSetPolicy::adjustInputs(TempAllocator& alloc, MInstruction* ins)
     _(StoreUnboxedScalarPolicy)                 \
     _(StoreUnboxedObjectOrNullPolicy)           \
     _(TestPolicy)                               \
-    _(AllDoublePolicy)                          \
     _(ToDoublePolicy)                           \
     _(ToInt32Policy)                            \
     _(ToStringPolicy)                           \

js/src/jit/VMFunctions.cpp

@@ -1357,5 +1357,19 @@ CheckIsCallable(JSContext* cx, HandleValue v, CheckIsCallableKind kind)
     return true;
 }
 
+template <bool allowBigInt = false>
+static bool DoToNumeric(JSContext* cx, HandleValue arg, MutableHandleValue ret)
+{
+    ret.set(arg);
+    if (allowBigInt) {
+        return ToNumeric(cx, ret);
+    }
+    return ToNumber(cx, ret);
+}
+
+typedef bool (*ToNumericFn)(JSContext*, HandleValue, MutableHandleValue);
+const VMFunction ToNumberInfo = FunctionInfo<ToNumericFn>(DoToNumeric, "ToNumber");
+const VMFunction ToNumericInfo = FunctionInfo<ToNumericFn>(DoToNumeric<true>, "ToNumeric");
+
 } // namespace jit
 } // namespace js

js/src/jit/VMFunctions.h

@@ -803,6 +803,9 @@ BaselineGetFunctionThis(JSContext* cx, BaselineFrame* frame, MutableHandleValue
 MOZ_MUST_USE bool
 CheckIsCallable(JSContext* cx, HandleValue v, CheckIsCallableKind kind);
 
+extern const VMFunction ToNumberInfo;
+extern const VMFunction ToNumericInfo;
+
 } // namespace jit
 } // namespace js
 

js/src/jit/shared/LIR-shared.h

@@ -7494,6 +7494,21 @@ class LLoadUnboxedExpando : public LInstructionHelper<1, 1, 0>
     }
 };
 
+// Ensure that a value is numeric, possibly via a VM call-out that invokes
+// valueOf().
+class LToNumeric : public LInstructionHelper<BOX_PIECES, BOX_PIECES, 0> {
+  public:
+    LIR_HEADER(ToNumeric)
+
+    explicit LToNumeric(const LBoxAllocation& input) {
+        setBoxOperand(Input, input);
+    }
+
+    static const size_t Input = 0;
+
+    const MToNumeric* mir() const { return mir_->toToNumeric(); }
+};
+
 // Guard that a value is in a TypeSet.
 class LTypeBarrierV : public LInstructionHelper<0, BOX_PIECES, 1>
 {

js/src/jit/shared/LOpcodes-shared.h

@@ -258,6 +258,7 @@
     _(GuardClass)                   \
     _(GuardUnboxedExpando)          \
     _(LoadUnboxedExpando)           \
+    _(ToNumeric)                    \
     _(TypeBarrierV)                 \
     _(TypeBarrierO)                 \
     _(MonitorTypes)                 \

js/src/vm/Interpreter.cpp

@@ -4196,6 +4196,14 @@ CASE(JSOP_DEC)
 }
 END_CASE(JSOP_DEC)
 
+CASE(JSOP_TONUMERIC)
+{
+    if (!ToNumeric(cx, REGS.stackHandleAt(-1))) {
+        goto error;
+    }
+}
+END_CASE(JSOP_TONUMERIC)
+
 CASE(JSOP_BIGINT)
 {
     PUSH_COPY(script->getConst(GET_UINT32_INDEX(REGS.pc)));

js/src/vm/Opcodes.h

@@ -2357,7 +2357,7 @@
      *   Operands:
      *   Stack: arg => rval
      */ \
-    macro(JSOP_DYNAMIC_IMPORT, 234, "call-import", NULL, 1,  1,  1,  JOF_BYTE) \
+    macro(JSOP_DYNAMIC_IMPORT, 234, "call-import", NULL, 1, 1, 1, JOF_BYTE) \
     /*
      * Pops the numeric value 'val' from the stack, then pushes 'val + 1'.
      *
@@ -2376,6 +2376,14 @@
      *   Stack: val => (val - 1)
      */ \
     macro(JSOP_DEC, 236, "dec", NULL, 1, 1, 1, JOF_BYTE) \
+    /*
+     * Pop 'val' from the stack, then push the result of 'ToNumeric(val)'.
+     *   Category: Operators
+     *   Type: Arithmetic Operators
+     *   Operands:
+     *   Stack: val => ToNumeric(val)
+     */ \
+    macro(JSOP_TONUMERIC, 237, "tonumeric", NULL, 1, 1, 1, JOF_BYTE) \
     /*
      * Pushes a BigInt constant onto the stack.
      *   Category: Literals
@@ -2383,13 +2391,12 @@
      *   Operands: uint32_t constIndex
      *   Stack: => val
      */ \
-    macro(JSOP_BIGINT, 237, "bigint", NULL,      5, 0, 1, JOF_BIGINT)
+    macro(JSOP_BIGINT, 238, "bigint", NULL, 5, 0, 1, JOF_BIGINT)
 /*
  * In certain circumstances it may be useful to "pad out" the opcode space to
  * a power of two.  Use this macro to do so.
  */
 #define FOR_EACH_TRAILING_UNUSED_OPCODE(macro) \
-    macro(238) \
     macro(239) \
     macro(240) \
     macro(241) \