Bug 1160884 - Add KeepAlive instructions after elements/slots uses. r=nbp

js/src/jit/CodeGenerator.cpp

@@ -2183,6 +2183,12 @@ CodeGenerator::visitNurseryObject(LNurseryObject* lir)
     masm.movePtr(ImmGCPtr(IonNurseryPtr(ptr)), output);
 }
 
+void
+CodeGenerator::visitKeepAliveObject(LKeepAliveObject* lir)
+{
+    // No-op.
+}
+
 void
 CodeGenerator::visitSlots(LSlots* lir)
 {

js/src/jit/CodeGenerator.h

@@ -108,6 +108,7 @@ class CodeGenerator : public CodeGeneratorSpecific
     void visitLambdaForSingleton(LLambdaForSingleton* lir);
     void visitPointer(LPointer* lir);
     void visitNurseryObject(LNurseryObject* lir);
+    void visitKeepAliveObject(LKeepAliveObject* lir);
     void visitSlots(LSlots* lir);
     void visitLoadSlotT(LLoadSlotT* lir);
     void visitLoadSlotV(LLoadSlotV* lir);

js/src/jit/Ion.cpp

@@ -1501,6 +1501,13 @@ OptimizeMIR(MIRGenerator* mir)
         AssertGraphCoherency(graph);
     }
 
+    if (!mir->compilingAsmJS()) {
+        AutoTraceLog log(logger, TraceLogger_AddKeepAliveInstructions);
+        AddKeepAliveInstructions(graph);
+        IonSpewPass("Add KeepAlive Instructions");
+        AssertGraphCoherency(graph);
+    }
+
     return true;
 }
 

js/src/jit/IonAnalysis.cpp

@@ -2729,6 +2729,111 @@ jit::EliminateRedundantChecks(MIRGraph& graph)
     return true;
 }
 
+static bool
+NeedsKeepAlive(MInstruction* slotsOrElements, MInstruction* use)
+{
+    MOZ_ASSERT(slotsOrElements->type() == MIRType_Elements ||
+               slotsOrElements->type() == MIRType_Slots);
+
+    if (slotsOrElements->block() != use->block())
+        return true;
+
+    MBasicBlock* block = use->block();
+    MInstructionIterator iter(block->begin(slotsOrElements));
+    MOZ_ASSERT(*iter == slotsOrElements);
+    ++iter;
+
+    while (true) {
+        if (*iter == use)
+            return false;
+
+        switch (iter->op()) {
+          case MDefinition::Op_Nop:
+          case MDefinition::Op_Constant:
+          case MDefinition::Op_KeepAliveObject:
+          case MDefinition::Op_Unbox:
+          case MDefinition::Op_LoadSlot:
+          case MDefinition::Op_StoreSlot:
+          case MDefinition::Op_LoadFixedSlot:
+          case MDefinition::Op_StoreFixedSlot:
+          case MDefinition::Op_LoadElement:
+          case MDefinition::Op_StoreElement:
+          case MDefinition::Op_InitializedLength:
+          case MDefinition::Op_ArrayLength:
+          case MDefinition::Op_BoundsCheck:
+            iter++;
+            break;
+          default:
+            return true;
+        }
+    }
+
+    MOZ_CRASH("Unreachable");
+}
+
+void
+jit::AddKeepAliveInstructions(MIRGraph& graph)
+{
+    for (MBasicBlockIterator i(graph.begin()); i != graph.end(); i++) {
+        MBasicBlock* block = *i;
+
+        for (MInstructionIterator insIter(block->begin()); insIter != block->end(); insIter++) {
+            MInstruction* ins = *insIter;
+            if (ins->type() != MIRType_Elements && ins->type() != MIRType_Slots)
+                continue;
+
+            MDefinition* ownerObject;
+            switch (ins->op()) {
+              case MDefinition::Op_ConstantElements:
+                continue;
+              case MDefinition::Op_ConvertElementsToDoubles:
+                // EliminateRedundantChecks should have replaced all uses.
+                MOZ_ASSERT(!ins->hasUses());
+                continue;
+              case MDefinition::Op_Elements:
+              case MDefinition::Op_TypedArrayElements:
+              case MDefinition::Op_TypedObjectElements:
+                MOZ_ASSERT(ins->numOperands() == 1);
+                ownerObject = ins->getOperand(0);
+                break;
+              case MDefinition::Op_Slots:
+                ownerObject = ins->toSlots()->object();
+                break;
+              default:
+                MOZ_CRASH("Unexpected op");
+            }
+
+            MOZ_ASSERT(ownerObject->type() == MIRType_Object);
+
+            for (MUseDefIterator uses(ins); uses; uses++) {
+                MInstruction* use = uses.def()->toInstruction();
+
+                if (use->isStoreElementHole()) {
+                    // StoreElementHole has an explicit object operand. If GVN
+                    // is disabled, we can get different unbox instructions with
+                    // the same object as input, so we check for that case.
+                    MOZ_ASSERT_IF(!use->toStoreElementHole()->object()->isUnbox() && !ownerObject->isUnbox(),
+                                  use->toStoreElementHole()->object() == ownerObject);
+                    continue;
+                }
+
+                if (use->isInArray()) {
+                    // See StoreElementHole case above.
+                    MOZ_ASSERT_IF(!use->toInArray()->object()->isUnbox() && !ownerObject->isUnbox(),
+                                  use->toInArray()->object() == ownerObject);
+                    continue;
+                }
+
+                if (!NeedsKeepAlive(ins, use))
+                    continue;
+
+                MKeepAliveObject* keepAlive = MKeepAliveObject::New(graph.alloc(), ownerObject);
+                use->block()->insertAfter(use, keepAlive);
+            }
+        }
+    }
+}
+
 bool
 LinearSum::multiply(int32_t scale)
 {

js/src/jit/IonAnalysis.h

@@ -83,6 +83,9 @@ AssertExtendedGraphCoherency(MIRGraph& graph);
 bool
 EliminateRedundantChecks(MIRGraph& graph);
 
+void
+AddKeepAliveInstructions(MIRGraph& graph);
+
 class MDefinition;
 
 // Simple linear sum of the form 'n' or 'x + n'.

js/src/jit/LIR-Common.h

@@ -4046,6 +4046,20 @@ class LLambdaArrow : public LInstructionHelper<1, 1 + BOX_PIECES, 1>
     }
 };
 
+class LKeepAliveObject : public LInstructionHelper<0, 1, 0>
+{
+  public:
+    LIR_HEADER(KeepAliveObject)
+
+    explicit LKeepAliveObject(const LAllocation& object) {
+        setOperand(0, object);
+    }
+
+    const LAllocation* object() {
+        return getOperand(0);
+    }
+};
+
 // Load the "slots" member out of a JSObject.
 //   Input: JSObject pointer
 //   Output: slots pointer

js/src/jit/LOpcodes.h

@@ -191,6 +191,7 @@
     _(Lambda)                       \
     _(LambdaArrow)                  \
     _(LambdaForSingleton)           \
+    _(KeepAliveObject)              \
     _(Slots)                        \
     _(Elements)                     \
     _(ConvertElementsToDoubles)     \

js/src/jit/Lowering.cpp

@@ -2171,6 +2171,15 @@ LIRGenerator::visitLambdaArrow(MLambdaArrow* ins)
     assignSafepoint(lir, ins);
 }
 
+void
+LIRGenerator::visitKeepAliveObject(MKeepAliveObject* ins)
+{
+    MDefinition* obj = ins->object();
+    MOZ_ASSERT(obj->type() == MIRType_Object);
+
+    add(new(alloc()) LKeepAliveObject(useKeepalive(obj)), ins);
+}
+
 void
 LIRGenerator::visitSlots(MSlots* ins)
 {

js/src/jit/Lowering.h

@@ -162,6 +162,7 @@ class LIRGenerator : public LIRGeneratorSpecific
     void visitStringReplace(MStringReplace* ins);
     void visitLambda(MLambda* ins);
     void visitLambdaArrow(MLambdaArrow* ins);
+    void visitKeepAliveObject(MKeepAliveObject* ins);
     void visitSlots(MSlots* ins);
     void visitElements(MElements* ins);
     void visitConstantElements(MConstantElements* ins);

js/src/jit/MIR.h

@@ -8212,6 +8212,29 @@ class MSetTypedObjectOffset
     }
 };
 
+class MKeepAliveObject
+  : public MUnaryInstruction,
+    public SingleObjectPolicy::Data
+{
+    explicit MKeepAliveObject(MDefinition* object)
+      : MUnaryInstruction(object)
+    {
+        setResultType(MIRType_None);
+        setGuard();
+    }
+
+  public:
+    INSTRUCTION_HEADER(KeepAliveObject)
+
+    static MKeepAliveObject* New(TempAllocator& alloc, MDefinition* object) {
+        return new(alloc) MKeepAliveObject(object);
+    }
+
+    MDefinition* object() const {
+        return getOperand(0);
+    }
+};
+
 // Perform !-operation
 class MNot
   : public MUnaryInstruction,

js/src/jit/MOpcodes.h

@@ -142,6 +142,7 @@ namespace jit {
     _(StringReplace)                                                        \
     _(Lambda)                                                               \
     _(LambdaArrow)                                                          \
+    _(KeepAliveObject)                                                      \
     _(Slots)                                                                \
     _(Elements)                                                             \
     _(ConstantElements)                                                     \

js/src/jit/shared/Lowering-shared-inl.h

@@ -358,12 +358,18 @@ LIRGeneratorShared::useStorableAtStart(MDefinition* mir)
 
 #endif
 
+LAllocation
+LIRGeneratorShared::useKeepalive(MDefinition* mir)
+{
+    return use(mir, LUse(LUse::KEEPALIVE));
+}
+
 LAllocation
 LIRGeneratorShared::useKeepaliveOrConstant(MDefinition* mir)
 {
     if (mir->isConstant())
         return LAllocation(mir->toConstant()->vp());
-    return use(mir, LUse(LUse::KEEPALIVE));
+    return useKeepalive(mir);
 }
 
 LUse

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

@@ -103,6 +103,7 @@ class LIRGeneratorShared : public MDefinitionVisitor
     // we can expect to write into memory in 1 instruction".
     inline LAllocation useStorable(MDefinition* mir);
     inline LAllocation useStorableAtStart(MDefinition* mir);
+    inline LAllocation useKeepalive(MDefinition* mir);
     inline LAllocation useKeepaliveOrConstant(MDefinition* mir);
     inline LAllocation useRegisterOrConstant(MDefinition* mir);
     inline LAllocation useRegisterOrConstantAtStart(MDefinition* mir);

js/src/vm/TraceLoggingTypes.h

@@ -55,6 +55,7 @@
     _(EliminateDeadCode)                              \
     _(EdgeCaseAnalysis)                               \
     _(EliminateRedundantChecks)                       \
+    _(AddKeepAliveInstructions)                       \
     _(GenerateLIR)                                    \
     _(RegisterAllocation)                             \
     _(GenerateCode)