Reconstruct operands when bailing out from add/sub overflows, bug 773666. r=jandem

js/src/ion/LIR-Common.h

@@ -1451,15 +1451,41 @@ class LMathFunctionD : public LCallInstructionHelper<1, 1, 1>
 // Adds two integers, returning an integer value.
 class LAddI : public LBinaryMath<0>
 {
+    bool recoversInput_;
+
   public:
     LIR_HEADER(AddI);
+
+    LAddI()
+      : recoversInput_(false)
+    { }
+
+    virtual bool recoversInput() const {
+        return recoversInput_;
+    }
+    void setRecoversInput() {
+        recoversInput_ = true;
+    }
 };
 
 // Subtracts two integers, returning an integer value.
 class LSubI : public LBinaryMath<0>
 {
+    bool recoversInput_;
+
   public:
     LIR_HEADER(SubI);
+
+    LSubI()
+      : recoversInput_(false)
+    { }
+
+    virtual bool recoversInput() const {
+        return recoversInput_;
+    }
+    void setRecoversInput() {
+        recoversInput_ = true;
+    }
 };
 
 // Performs an add, sub, mul, or div on two double values.

js/src/ion/LIR.cpp

@@ -130,6 +130,17 @@ LSnapshot::New(MIRGenerator *gen, MResumePoint *mir, BailoutKind kind)
     return snapshot;
 }
 
+void
+LSnapshot::rewriteRecoveredInput(LUse input)
+{
+    // Mark any operands to this snapshot with the same value as input as being
+    // equal to the instruction's result.
+    for (size_t i = 0; i < numEntries(); i++) {
+        if (getEntry(i)->isUse() && getEntry(i)->toUse()->virtualRegister() == input.virtualRegister())
+            setEntry(i, LUse(input.virtualRegister(), LUse::RECOVERED_INPUT));
+    }
+}
+
 bool
 LPhi::init(MIRGenerator *gen)
 {

js/src/ion/LIR.h

@@ -234,7 +234,14 @@ class LUse : public LAllocation
         // Keep the used virtual register alive, and use whatever allocation is
         // available. This is similar to ANY but hints to the register allocator
         // that it is never useful to optimize this site.
-        KEEPALIVE
+        KEEPALIVE,
+
+        // For snapshot inputs, indicates that the associated instruction will
+        // write this input to its output register before bailing out.
+        // The register allocator may thus allocate that output register, and
+        // does not need to keep the virtual register alive (alternatively,
+        // this may be treated as KEEPALIVE).
+        RECOVERED_INPUT
     };
 
     void set(Policy policy, uint32 reg, bool usedAtStart) {
@@ -641,6 +648,12 @@ class LInstruction
     void assignSnapshot(LSnapshot *snapshot);
     void initSafepoint();
 
+    // For an instruction which has a MUST_REUSE_INPUT output, whether that
+    // output register will be restored to its original value when bailing out.
+    virtual bool recoversInput() const {
+        return false;
+    }
+
     virtual void print(FILE *fp);
     static void printName(FILE *fp, Opcode op);
     virtual void printName(FILE *fp);
@@ -898,6 +911,7 @@ class LSnapshot : public TempObject
     void setBailoutKind(BailoutKind kind) {
         bailoutKind_ = kind;
     }
+    void rewriteRecoveredInput(LUse input);
 };
 
 struct SafepointNunboxEntry {

js/src/ion/LinearScan.cpp

@@ -325,8 +325,12 @@ UsePosition *
 LiveInterval::nextUseAfter(CodePosition after)
 {
     for (UsePositionIterator usePos(usesBegin()); usePos != usesEnd(); usePos++) {
-        if (usePos->pos >= after && usePos->use->policy() != LUse::KEEPALIVE)
-            return *usePos;
+        if (usePos->pos >= after) {
+            LUse::Policy policy = usePos->use->policy();
+            JS_ASSERT(policy != LUse::RECOVERED_INPUT);
+            if (policy != LUse::KEEPALIVE)
+                return *usePos;
+        }
     }
     return NULL;
 }
@@ -674,6 +678,10 @@ LinearScanAllocator::buildLivenessInfo()
                     JS_ASSERT(!(hasUseRegister && hasUseRegisterAtStart));
 #endif
 
+                    // Don't treat RECOVERED_INPUT uses as keeping the vreg alive.
+                    if (use->policy() == LUse::RECOVERED_INPUT)
+                        continue;
+
                     CodePosition to;
                     if (use->isFixedRegister()) {
                         JS_ASSERT(!use->usedAtStart());
@@ -1129,6 +1137,15 @@ LinearScanAllocator::reifyAllocations()
                 spillFrom = interval->getAllocation();
             }
 
+            if (reg->ins()->recoversInput()) {
+                LSnapshot *snapshot = reg->ins()->snapshot();
+                for (size_t i = 0; i < snapshot->numEntries(); i++) {
+                    LAllocation *entry = snapshot->getEntry(i);
+                    if (entry->isUse() && entry->toUse()->policy() == LUse::RECOVERED_INPUT)
+                        *entry = *def->output();
+                }
+            }
+
             if (reg->mustSpillAtDefinition() && !reg->ins()->isPhi() &&
                 (*reg->canonicalSpill() != *spillFrom))
             {

js/src/ion/Lowering.cpp

@@ -792,6 +792,38 @@ LIRGenerator::visitMathFunction(MMathFunction *ins)
     return defineReturn(lir, ins);
 }
 
+// Try to mark an add or sub instruction as able to recover its input when
+// bailing out.
+template <typename S, typename T>
+static void
+MaybeSetRecoversInput(S *mir, T *lir)
+{
+    JS_ASSERT(lir->mirRaw() == mir);
+    if (!mir->fallible())
+        return;
+
+    if (lir->output()->policy() != LDefinition::MUST_REUSE_INPUT)
+        return;
+
+    // The original operands to an add or sub can't be recovered if they both
+    // use the same register.
+    if (lir->lhs()->isUse() && lir->rhs()->isUse() &&
+        lir->lhs()->toUse()->virtualRegister() == lir->rhs()->toUse()->virtualRegister())
+    {
+        return;
+    }
+
+    // Add instructions that are on two different values can recover
+    // the input they clobbered via MUST_REUSE_INPUT. Thus, a copy
+    // of that input does not need to be kept alive in the snapshot
+    // for the instruction.
+
+    lir->setRecoversInput();
+
+    const LUse *input = lir->getOperand(lir->output()->getReusedInput())->toUse();
+    lir->snapshot()->rewriteRecoveredInput(*input);
+}
+
 bool
 LIRGenerator::visitAdd(MAdd *ins)
 {
@@ -804,10 +836,15 @@ LIRGenerator::visitAdd(MAdd *ins)
         JS_ASSERT(lhs->type() == MIRType_Int32);
         ReorderCommutative(&lhs, &rhs);
         LAddI *lir = new LAddI;
+
         if (ins->fallible() && !assignSnapshot(lir))
             return false;
 
-        return lowerForALU(lir, ins, lhs, rhs);
+        if (!lowerForALU(lir, ins, lhs, rhs))
+            return false;
+
+        MaybeSetRecoversInput(ins, lir);
+        return true;
     }
 
     if (ins->specialization() == MIRType_Double) {
@@ -828,11 +865,16 @@ LIRGenerator::visitSub(MSub *ins)
 
     if (ins->specialization() == MIRType_Int32) {
         JS_ASSERT(lhs->type() == MIRType_Int32);
+
         LSubI *lir = new LSubI;
         if (ins->fallible() && !assignSnapshot(lir))
             return false;
 
-        return lowerForALU(lir, ins, lhs, rhs);
+        if (!lowerForALU(lir, ins, lhs, rhs))
+            return false;
+
+        MaybeSetRecoversInput(ins, lir);
+        return true;
     }
     if (ins->specialization() == MIRType_Double) {
         JS_ASSERT(lhs->type() == MIRType_Double);

js/src/ion/shared/CodeGenerator-x86-shared.cpp

@@ -508,6 +508,23 @@ CodeGeneratorX86Shared::visitPowHalfD(LPowHalfD *ins)
     return true;
 }
 
+class OutOfLineUndoALUOperation : public OutOfLineCodeBase<CodeGeneratorX86Shared>
+{
+    LInstruction *ins_;
+
+  public:
+    OutOfLineUndoALUOperation(LInstruction *ins)
+        : ins_(ins)
+    { }
+
+    virtual bool accept(CodeGeneratorX86Shared *codegen) {
+        return codegen->visitOutOfLineUndoALUOperation(this);
+    }
+    LInstruction *ins() const {
+        return ins_;
+    }
+};
+
 bool
 CodeGeneratorX86Shared::visitAddI(LAddI *ins)
 {
@@ -516,8 +533,17 @@ CodeGeneratorX86Shared::visitAddI(LAddI *ins)
     else
         masm.addl(ToOperand(ins->rhs()), ToRegister(ins->lhs()));
 
-    if (ins->snapshot() && !bailoutIf(Assembler::Overflow, ins->snapshot()))
-        return false;
+    if (ins->snapshot()) {
+        if (ins->recoversInput()) {
+            OutOfLineUndoALUOperation *ool = new OutOfLineUndoALUOperation(ins);
+            if (!addOutOfLineCode(ool))
+                return false;
+            masm.j(Assembler::Overflow, ool->entry());
+        } else {
+            if (!bailoutIf(Assembler::Overflow, ins->snapshot()))
+                return false;
+        }
+    }
     return true;
 }
 
@@ -529,11 +555,54 @@ CodeGeneratorX86Shared::visitSubI(LSubI *ins)
     else
         masm.subl(ToOperand(ins->rhs()), ToRegister(ins->lhs()));
 
-    if (ins->snapshot() && !bailoutIf(Assembler::Overflow, ins->snapshot()))
-        return false;
+    if (ins->snapshot()) {
+        if (ins->recoversInput()) {
+            OutOfLineUndoALUOperation *ool = new OutOfLineUndoALUOperation(ins);
+            if (!addOutOfLineCode(ool))
+                return false;
+            masm.j(Assembler::Overflow, ool->entry());
+        } else {
+            if (!bailoutIf(Assembler::Overflow, ins->snapshot()))
+                return false;
+        }
+    }
     return true;
 }
 
+bool
+CodeGeneratorX86Shared::visitOutOfLineUndoALUOperation(OutOfLineUndoALUOperation *ool)
+{
+    LInstruction *ins = ool->ins();
+    Register reg = ToRegister(ins->getDef(0));
+
+    LAllocation *lhs = ins->getOperand(0);
+    LAllocation *rhs = ins->getOperand(1);
+
+    JS_ASSERT(reg == ToRegister(lhs));
+    JS_ASSERT_IF(rhs->isGeneralReg(), reg != ToRegister(rhs));
+
+    // Undo the effect of the ALU operation, which was performed on the output
+    // register and overflowed. Writing to the output register clobbered an
+    // input reg, and the original value of the input needs to be recovered
+    // to satisfy the constraint imposed by any RECOVERED_INPUT operands to
+    // the bailout snapshot.
+
+    if (rhs->isConstant()) {
+        Imm32 constant(ToInt32(rhs));
+        if (ins->isAddI())
+            masm.subl(constant, reg);
+        else
+            masm.addl(constant, reg);
+    } else {
+        if (ins->isAddI())
+            masm.subl(ToOperand(rhs), reg);
+        else
+            masm.addl(ToOperand(rhs), reg);
+    }
+
+    return bailout(ool->ins()->snapshot());
+}
+
 class MulNegativeZeroCheck : public OutOfLineCodeBase<CodeGeneratorX86Shared>
 {
     LMulI *ins_;

js/src/ion/shared/CodeGenerator-x86-shared.h

@@ -14,6 +14,7 @@ namespace js {
 namespace ion {
 
 class OutOfLineBailout;
+class OutOfLineUndoALUOperation;
 class MulNegativeZeroCheck;
 class OutOfLineTruncate;
 
@@ -120,6 +121,7 @@ class CodeGeneratorX86Shared : public CodeGeneratorShared
 
     // Out of line visitors.
     bool visitOutOfLineBailout(OutOfLineBailout *ool);
+    bool visitOutOfLineUndoALUOperation(OutOfLineUndoALUOperation *ool);
     bool visitMulNegativeZeroCheck(MulNegativeZeroCheck *ool);
     bool visitOutOfLineTruncate(OutOfLineTruncate *ool);
     bool generateInvalidateEpilogue();