Bug 1160884 - Add KeepAlive instructions after elements/slots uses. r=nbp, a=abillings
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 28 May 2015 10:16:24 +0200
changeset 201202 266dc3dff7395ca415f8094d92a997aacbc32a41
parent 201201 088d28723fcd1b04ce933316c03422b2561bac17
child 201203 772f5b18522c7351c5897ae12087a0319a1c0ac6
push id278
push userjandemooij@gmail.com
push dateWed, 03 Jun 2015 10:37:54 +0000
treeherdermozilla-esr31@266dc3dff739 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp, abillings
bugs1160884
milestone31.7.0
Bug 1160884 - Add KeepAlive instructions after elements/slots uses. r=nbp, a=abillings
js/src/jit/CodeGenerator.cpp
js/src/jit/CodeGenerator.h
js/src/jit/Ion.cpp
js/src/jit/IonAnalysis.cpp
js/src/jit/IonAnalysis.h
js/src/jit/LIR-Common.h
js/src/jit/LOpcodes.h
js/src/jit/Lowering.cpp
js/src/jit/Lowering.h
js/src/jit/MIR.h
js/src/jit/MOpcodes.h
js/src/jit/ParallelSafetyAnalysis.cpp
js/src/jit/shared/Lowering-shared-inl.h
js/src/jit/shared/Lowering-shared.h
js/src/vm/TraceLogging.h
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -1471,16 +1471,23 @@ CodeGenerator::visitPointer(LPointer* li
     if (lir->kind() == LPointer::GC_THING)
         masm.movePtr(ImmGCPtr(lir->gcptr()), ToRegister(lir->output()));
     else
         masm.movePtr(ImmPtr(lir->ptr()), ToRegister(lir->output()));
     return true;
 }
 
 bool
+CodeGenerator::visitKeepAliveObject(LKeepAliveObject* lir)
+{
+    // No-op.
+    return true;
+}
+
+bool
 CodeGenerator::visitSlots(LSlots* lir)
 {
     Address slots(ToRegister(lir->object()), JSObject::offsetOfSlots());
     masm.loadPtr(slots, ToRegister(lir->output()));
     return true;
 }
 
 bool
--- a/js/src/jit/CodeGenerator.h
+++ b/js/src/jit/CodeGenerator.h
@@ -101,16 +101,17 @@ class CodeGenerator : public CodeGenerat
     bool visitRegExpTest(LRegExpTest* lir);
     bool visitRegExpReplace(LRegExpReplace* lir);
     bool visitStringReplace(LStringReplace* lir);
     bool visitLambda(LLambda* lir);
     bool visitLambdaArrow(LLambdaArrow* lir);
     bool visitLambdaForSingleton(LLambdaForSingleton* lir);
     bool visitLambdaPar(LLambdaPar* lir);
     bool visitPointer(LPointer* lir);
+    bool visitKeepAliveObject(LKeepAliveObject* lir);
     bool visitSlots(LSlots* lir);
     bool visitStoreSlotV(LStoreSlotV* store);
     bool visitElements(LElements* lir);
     bool visitConvertElementsToDoubles(LConvertElementsToDoubles* lir);
     bool visitMaybeToDoubleElement(LMaybeToDoubleElement* lir);
     bool visitGuardObjectIdentity(LGuardObjectIdentity* guard);
     bool visitTypeBarrierV(LTypeBarrierV* lir);
     bool visitTypeBarrierO(LTypeBarrierO* lir);
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -1531,16 +1531,23 @@ OptimizeMIR(MIRGenerator* mir)
         // code motion after this pass could incorrectly move a load or store
         // before its bounds check.
         if (!EliminateRedundantChecks(graph))
             return false;
         IonSpewPass("Bounds Check Elimination");
         AssertGraphCoherency(graph);
     }
 
+    if (!mir->compilingAsmJS()) {
+        AutoTraceLog log(logger, TraceLogger::AddKeepAliveInstructions);
+        AddKeepAliveInstructions(graph);
+        IonSpewPass("Add KeepAlive Instructions");
+        AssertGraphCoherency(graph);
+    }
+
     return true;
 }
 
 LIRGraph*
 GenerateLIR(MIRGenerator* mir)
 {
     MIRGraph& graph = mir->graph();
 
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -1966,16 +1966,128 @@ jit::UnsplitEdges(LIRGraph* lir)
         lir->removeBlock(i);
         lir->mir().removeBlock(mirBlock);
         --i;
     }
 
     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:
+              case MDefinition::Op_NewSlots:
+                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);
+
+            if (ownerObject->isConstant()) {
+                // Constants are kept alive by other pointers, for instance
+                // ImmGCPtr in JIT code.
+                continue;
+            }
+
+            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)
 {
     for (size_t i = 0; i < terms_.length(); i++) {
         if (!SafeMul(scale, terms_[i].scale, &terms_[i].scale))
             return false;
     }
     return SafeMul(scale, constant_, &constant_);
--- a/js/src/jit/IonAnalysis.h
+++ b/js/src/jit/IonAnalysis.h
@@ -59,16 +59,19 @@ void
 AssertGraphCoherency(MIRGraph& graph);
 
 void
 AssertExtendedGraphCoherency(MIRGraph& graph);
 
 bool
 EliminateRedundantChecks(MIRGraph& graph);
 
+void
+AddKeepAliveInstructions(MIRGraph& graph);
+
 bool
 UnsplitEdges(LIRGraph* lir);
 
 class MDefinition;
 
 // Simple linear sum of the form 'n' or 'x + n'.
 struct SimpleLinearSum
 {
--- a/js/src/jit/LIR-Common.h
+++ b/js/src/jit/LIR-Common.h
@@ -3586,16 +3586,30 @@ class LImplicitThis : public LInstructio
     const MImplicitThis* mir() const {
         return mir_->toImplicitThis();
     }
     const LAllocation* callee() {
         return getOperand(0);
     }
 };
 
+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
 class LSlots : public LInstructionHelper<1, 1, 0>
 {
   public:
     LIR_HEADER(Slots)
 
--- a/js/src/jit/LOpcodes.h
+++ b/js/src/jit/LOpcodes.h
@@ -161,16 +161,17 @@
     _(RegExpTest)                   \
     _(RegExpReplace)                \
     _(StringReplace)                \
     _(Lambda)                       \
     _(LambdaArrow)                  \
     _(LambdaForSingleton)           \
     _(LambdaPar)                    \
     _(ImplicitThis)                 \
+    _(KeepAliveObject)              \
     _(Slots)                        \
     _(Elements)                     \
     _(ConvertElementsToDoubles)     \
     _(MaybeToDoubleElement)         \
     _(LoadSlotV)                    \
     _(LoadSlotT)                    \
     _(StoreSlotV)                   \
     _(StoreSlotT)                   \
--- a/js/src/jit/Lowering.cpp
+++ b/js/src/jit/Lowering.cpp
@@ -2105,16 +2105,25 @@ LIRGenerator::visitImplicitThis(MImplici
 {
     JS_ASSERT(ins->callee()->type() == MIRType_Object);
 
     LImplicitThis* lir = new(alloc()) LImplicitThis(useRegister(ins->callee()));
     return assignSnapshot(lir) && defineBox(lir, ins);
 }
 
 bool
+LIRGenerator::visitKeepAliveObject(MKeepAliveObject* ins)
+{
+    MDefinition* obj = ins->object();
+    MOZ_ASSERT(obj->type() == MIRType_Object);
+
+    return add(new(alloc()) LKeepAliveObject(useKeepalive(obj)), ins);
+}
+
+bool
 LIRGenerator::visitSlots(MSlots* ins)
 {
     return define(new(alloc()) LSlots(useRegisterAtStart(ins->object())), ins);
 }
 
 bool
 LIRGenerator::visitElements(MElements* ins)
 {
--- a/js/src/jit/Lowering.h
+++ b/js/src/jit/Lowering.h
@@ -155,16 +155,17 @@ class LIRGenerator : public LIRGenerator
     bool visitRegExpExec(MRegExpExec* ins);
     bool visitRegExpTest(MRegExpTest* ins);
     bool visitRegExpReplace(MRegExpReplace* ins);
     bool visitStringReplace(MStringReplace* ins);
     bool visitLambda(MLambda* ins);
     bool visitLambdaArrow(MLambdaArrow* ins);
     bool visitLambdaPar(MLambdaPar* ins);
     bool visitImplicitThis(MImplicitThis* ins);
+    bool visitKeepAliveObject(MKeepAliveObject* ins);
     bool visitSlots(MSlots* ins);
     bool visitElements(MElements* ins);
     bool visitConstantElements(MConstantElements* ins);
     bool visitConvertElementsToDoubles(MConvertElementsToDoubles* ins);
     bool visitMaybeToDoubleElement(MMaybeToDoubleElement* ins);
     bool visitLoadSlot(MLoadSlot* ins);
     bool visitFunctionEnvironment(MFunctionEnvironment* ins);
     bool visitForkJoinContext(MForkJoinContext* ins);
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -5785,16 +5785,42 @@ class MSetTypedObjectOffset
 
     AliasSet getAliasSet() const {
         // This affects the result of MTypedObjectElements,
         // which is described as a load of ObjectFields.
         return AliasSet::Store(AliasSet::ObjectFields);
     }
 };
 
+class MKeepAliveObject
+  : public MUnaryInstruction,
+    public SingleObjectPolicy
+{
+    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);
+    }
+    TypePolicy* typePolicy() {
+        return this;
+    }
+};
+
 // Perform !-operation
 class MNot
   : public MUnaryInstruction,
     public TestPolicy
 {
     bool operandMightEmulateUndefined_;
     bool operandIsNeverNaN_;
 
--- a/js/src/jit/MOpcodes.h
+++ b/js/src/jit/MOpcodes.h
@@ -105,16 +105,17 @@ namespace jit {
     _(RegExp)                                                               \
     _(RegExpExec)                                                           \
     _(RegExpTest)                                                           \
     _(RegExpReplace)                                                        \
     _(StringReplace)                                                        \
     _(Lambda)                                                               \
     _(LambdaArrow)                                                          \
     _(ImplicitThis)                                                         \
+    _(KeepAliveObject)                                                      \
     _(Slots)                                                                \
     _(Elements)                                                             \
     _(ConstantElements)                                                     \
     _(ConvertElementsToDoubles)                                             \
     _(MaybeToDoubleElement)                                                 \
     _(LoadSlot)                                                             \
     _(StoreSlot)                                                            \
     _(FunctionEnvironment)                                                  \
--- a/js/src/jit/ParallelSafetyAnalysis.cpp
+++ b/js/src/jit/ParallelSafetyAnalysis.cpp
@@ -194,16 +194,17 @@ class ParallelSafetyVisitor : public MIn
     UNSAFE_OP(InitPropGetterSetter)
     SAFE_OP(Start)
     UNSAFE_OP(OsrEntry)
     SAFE_OP(Nop)
     UNSAFE_OP(RegExp)
     CUSTOM_OP(Lambda)
     UNSAFE_OP(LambdaArrow)
     UNSAFE_OP(ImplicitThis)
+    SAFE_OP(KeepAliveObject)
     SAFE_OP(Slots)
     SAFE_OP(Elements)
     SAFE_OP(ConstantElements)
     SAFE_OP(LoadSlot)
     WRITE_GUARDED_OP(StoreSlot, slots)
     SAFE_OP(FunctionEnvironment) // just a load of func env ptr
     SAFE_OP(FilterTypeSet)
     SAFE_OP(TypeBarrier) // causes a bailout if the type is not found: a-ok with us
--- a/js/src/jit/shared/Lowering-shared-inl.h
+++ b/js/src/jit/shared/Lowering-shared-inl.h
@@ -367,21 +367,27 @@ LAllocation
 LIRGeneratorShared::useStorableAtStart(MDefinition* mir)
 {
     return useRegisterOrConstantAtStart(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
 LIRGeneratorShared::useFixed(MDefinition* mir, Register reg)
 {
     return use(mir, LUse(reg));
 }
 
--- a/js/src/jit/shared/Lowering-shared.h
+++ b/js/src/jit/shared/Lowering-shared.h
@@ -80,16 +80,17 @@ class LIRGeneratorShared : public MInstr
     // and only registers on ARM.
     inline LAllocation useAny(MDefinition* mir);
     inline LAllocation useAnyOrConstant(MDefinition* mir);
     // "Storable" is architecture dependend, and will include registers and constants on X86
     // and only registers on ARM.
     // this is a generic "things 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);
     inline LAllocation useRegisterOrNonNegativeConstantAtStart(MDefinition* mir);
     inline LAllocation useRegisterOrNonDoubleConstant(MDefinition* mir);
 
 #ifdef JS_NUNBOX32
     inline LUse useType(MDefinition* mir, LUse::Policy policy);
--- a/js/src/vm/TraceLogging.h
+++ b/js/src/vm/TraceLogging.h
@@ -140,17 +140,18 @@ namespace jit {
     _(AliasAnalysis)                                  \
     _(GVN)                                            \
     _(UCE)                                            \
     _(LICM)                                           \
     _(RangeAnalysis)                                  \
     _(EffectiveAddressAnalysis)                       \
     _(EliminateDeadCode)                              \
     _(EdgeCaseAnalysis)                               \
-    _(EliminateRedundantChecks)
+    _(EliminateRedundantChecks)                       \
+    _(AddKeepAliveInstructions)
 
 class AutoTraceLog;
 
 template <class T>
 class ContinuousSpace {
     T* data_;
     uint32_t next_;
     uint32_t capacity_;