Insert additional resume points after operations likely to kill values, bug 814997. r=dvander

js/src/ion/IonAnalysis.cpp

@@ -143,7 +143,8 @@ ion::EliminateDeadCode(MIRGenerator *mir, MIRGraph &graph)
 
         // Remove unused instructions.
         for (MInstructionReverseIterator inst = block->rbegin(); inst != block->rend(); ) {
-            if (!inst->isEffectful() && !inst->hasUses() && !inst->isGuard() &&
+            if (!inst->isEffectful() && !inst->resumePoint() &&
+                !inst->hasUses() && !inst->isGuard() &&
                 !inst->isControlInstruction()) {
                 inst = block->discardAt(inst);
             } else {

js/src/ion/IonBuilder.cpp

@@ -900,7 +900,14 @@ IonBuilder::inspectOpcode(JSOp op)
 
       case JSOP_POP:
         current->pop();
-        return true;
+
+        // POP opcodes frequently appear where values are killed, e.g. after
+        // SET* opcodes. Place a resume point afterwards to avoid capturing
+        // the dead value in later snapshots, except in places where that
+        // resume point is obviously unnecessary.
+        if (pc[JSOP_POP_LENGTH] == JSOP_POP)
+            return true;
+        return maybeInsertResume();
 
       case JSOP_NEWINIT:
       {
@@ -2699,7 +2706,7 @@ IonBuilder::jsop_binary(JSOp op, MDefinition *left, MDefinition *right)
         MConcat *ins = MConcat::New(left, right);
         current->add(ins);
         current->push(ins);
-        return true;
+        return maybeInsertResume();
     }
 
     MBinaryArithInstruction *ins;
@@ -2736,7 +2743,7 @@ IonBuilder::jsop_binary(JSOp op, MDefinition *left, MDefinition *right)
 
     if (ins->isEffectful())
         return resumeAfter(ins);
-    return true;
+    return maybeInsertResume();
 }
 
 bool
@@ -4300,7 +4307,7 @@ IonBuilder::newPendingLoopHeader(MBasicBlock *predecessor, jsbytecode *pc)
 bool
 IonBuilder::resume(MInstruction *ins, jsbytecode *pc, MResumePoint::Mode mode)
 {
-    JS_ASSERT(ins->isEffectful());
+    JS_ASSERT(ins->isEffectful() || !ins->isMovable());
 
     MResumePoint *resumePoint = MResumePoint::New(ins->block(), pc, callerResumePoint_, mode);
     if (!resumePoint)
@@ -4322,6 +4329,28 @@ IonBuilder::resumeAfter(MInstruction *ins)
     return resume(ins, pc, MResumePoint::ResumeAfter);
 }
 
+bool
+IonBuilder::maybeInsertResume()
+{
+    // Create a resume point at the current position, without an existing
+    // effectful instruction. This resume point is not necessary for correct
+    // behavior (see above), but is added to avoid holding any values from the
+    // previous resume point which are now dead. This shortens the live ranges
+    // of such values and improves register allocation.
+    //
+    // This optimization is not performed outside of loop bodies, where good
+    // register allocation is not as critical, in order to avoid creating
+    // excessive resume points.
+
+    if (loopDepth_ == 0)
+        return true;
+
+    MNop *ins = MNop::New();
+    current->add(ins);
+
+    return resumeAfter(ins);
+}
+
 void
 IonBuilder::insertRecompileCheck()
 {

js/src/ion/IonBuilder.h

@@ -257,6 +257,7 @@ class IonBuilder : public MIRGenerator
     bool resume(MInstruction *ins, jsbytecode *pc, MResumePoint::Mode mode);
     bool resumeAt(MInstruction *ins, jsbytecode *pc);
     bool resumeAfter(MInstruction *ins);
+    bool maybeInsertResume();
 
     void insertRecompileCheck();
 

js/src/ion/Lowering.cpp

@@ -1032,6 +1032,12 @@ LIRGenerator::visitStart(MStart *start)
     return add(lir);
 }
 
+bool
+LIRGenerator::visitNop(MNop *nop)
+{
+    return true;
+}
+
 bool
 LIRGenerator::visitOsrEntry(MOsrEntry *entry)
 {

js/src/ion/Lowering.h

@@ -122,6 +122,7 @@ class LIRGenerator : public LIRGeneratorSpecific
     bool visitFromCharCode(MFromCharCode *ins);
     bool visitStart(MStart *start);
     bool visitOsrEntry(MOsrEntry *entry);
+    bool visitNop(MNop *nop);
     bool visitOsrValue(MOsrValue *value);
     bool visitOsrScopeChain(MOsrScopeChain *object);
     bool visitToDouble(MToDouble *convert);

js/src/ion/MIR.h

@@ -598,6 +598,25 @@ class MOsrEntry : public MNullaryInstruction
     }
 };
 
+// No-op instruction. This cannot be moved or eliminated, and is intended for
+// anchoring resume points at arbitrary points in a block.
+class MNop : public MNullaryInstruction
+{
+  protected:
+    MNop() {
+    }
+
+  public:
+    INSTRUCTION_HEADER(Nop);
+    static MNop *New() {
+        return new MNop();
+    }
+
+    AliasSet getAliasSet() const {
+        return AliasSet::None();
+    }
+};
+
 // A constant js::Value.
 class MConstant : public MNullaryInstruction
 {

js/src/ion/MOpcodes.h

@@ -76,6 +76,7 @@ namespace ion {
     _(InitProp)                                                             \
     _(Start)                                                                \
     _(OsrEntry)                                                             \
+    _(Nop)                                                                  \
     _(RegExp)                                                               \
     _(RegExpTest)                                                           \
     _(Lambda)                                                               \