Bug 991720 part 4 - Scalar replacement registers the state instead of replacing resume points operands. r=h4writer
authorNicolas B. Pierron <nicolas.b.pierron@mozilla.com>
Fri, 19 Dec 2014 15:28:31 +0100
changeset 220608 b421cdd4b918eeebacdb42debf9690d71bf0d397
parent 220607 f5497ebe2735a639bdd1c135260e9c55338c7015
child 220609 6367ccd704d3a0e24b521f3a848071b1db563ee8
push id10503
push userryanvm@gmail.com
push dateFri, 19 Dec 2014 20:13:42 +0000
treeherderfx-team@98ee95ac6be5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersh4writer
bugs991720
milestone37.0a1
Bug 991720 part 4 - Scalar replacement registers the state instead of replacing resume points operands. r=h4writer
js/src/jit/ScalarReplacement.cpp
--- a/js/src/jit/ScalarReplacement.cpp
+++ b/js/src/jit/ScalarReplacement.cpp
@@ -12,52 +12,16 @@
 #include "jit/JitSpewer.h"
 #include "jit/MIR.h"
 #include "jit/MIRGenerator.h"
 #include "jit/MIRGraph.h"
 
 namespace js {
 namespace jit {
 
-// Scan resume point operands in search of a local variable which captures the
-// current object, and replace it by the current object with its state.
-static void
-ReplaceResumePointOperands(MResumePoint *resumePoint, MDefinition *object, MDefinition *state)
-{
-    // Note: This function iterates over the caller as well, this is wrong
-    // because if the object appears in one of the caller, we want to correctly
-    // recover the object value from any block having the same caller.  In
-    // practice, this is correct for 2 reasons:
-    //
-    // 1. We replace resume point operands in RPO, this implies that the caller
-    // would first be updated when we update the resume point of entry block of
-    // the inner function.  This implies that the object state would only hold
-    // valid data for the caller resume point.
-    //
-    // 2. The caller resume point will have no reference of the new object
-    // allocation if the object allocation is done within the callee.
-    //
-    // A side-effect of this implementation is that we would be restoring and
-    // keeping tracks of the content of the object at the entry of the function,
-    // in addition to the content of the object within the function.
-    for (MResumePoint *rp = resumePoint; rp; rp = rp->caller()) {
-        for (size_t op = 0; op < rp->numOperands(); op++) {
-            if (rp->getOperand(op) == object) {
-                rp->replaceOperand(op, state);
-
-                // This assertion verifies the comment which is above still
-                // holds.  Note, this is not true if rp == resumePoint, as the
-                // object state can be a new one created at the beginning of the
-                // block to keep track of the merge state.
-                MOZ_ASSERT_IF(rp != resumePoint, state->block()->dominates(rp->block()));
-            }
-        }
-    }
-}
-
 template <typename MemoryView>
 class EmulateStateOf
 {
   private:
     typedef typename MemoryView::BlockState BlockState;
 
     MIRGenerator *mir_;
     MIRGraph &graph_;
@@ -248,16 +212,19 @@ class ObjectMemoryView : public MDefinit
 
   private:
     TempAllocator &alloc_;
     MConstant *undefinedVal_;
     MInstruction *obj_;
     MBasicBlock *startBlock_;
     BlockState *state_;
 
+    // Used to improve the memory usage by sharing common modification.
+    const MResumePoint *lastResumePoint_;
+
   public:
     ObjectMemoryView(TempAllocator &alloc, MInstruction *obj);
 
     MBasicBlock *startingBlock();
     bool initStartingState(BlockState **pState);
 
     void setEntryBlockState(BlockState *state);
     bool mergeIntoSuccessorState(MBasicBlock *curr, MBasicBlock *succ, BlockState **pSuccState);
@@ -265,32 +232,39 @@ class ObjectMemoryView : public MDefinit
 #ifdef DEBUG
     void assertSuccess();
 #else
     void assertSuccess() {}
 #endif
 
   public:
     void visitResumePoint(MResumePoint *rp);
+    void visitObjectState(MObjectState *ins);
     void visitStoreFixedSlot(MStoreFixedSlot *ins);
     void visitLoadFixedSlot(MLoadFixedSlot *ins);
     void visitStoreSlot(MStoreSlot *ins);
     void visitLoadSlot(MLoadSlot *ins);
     void visitGuardShape(MGuardShape *ins);
     void visitFunctionEnvironment(MFunctionEnvironment *ins);
+    void visitLambda(MLambda *ins);
 };
 
 const char *ObjectMemoryView::phaseName = "Scalar Replacement of Object";
 
 ObjectMemoryView::ObjectMemoryView(TempAllocator &alloc, MInstruction *obj)
   : alloc_(alloc),
     obj_(obj),
-    startBlock_(obj->block())
+    startBlock_(obj->block()),
+    state_(nullptr),
+    lastResumePoint_(nullptr)
 {
-    // Annoatte the instruction such that we do not replace it by a
+    // Annotate snapshots RValue such that we recover the store first.
+    obj_->setIncompleteObject();
+
+    // Annotate the instruction such that we do not replace it by a
     // Magic(JS_OPTIMIZED_OUT) in case of removed uses.
     obj_->setImplicitlyUsedUnchecked();
 }
 
 MBasicBlock *
 ObjectMemoryView::startingBlock()
 {
     return startBlock_;
@@ -302,16 +276,19 @@ ObjectMemoryView::initStartingState(Bloc
     // Uninitialized slots have an "undefined" value.
     undefinedVal_ = MConstant::New(alloc_, UndefinedValue());
     startBlock_->insertBefore(obj_, undefinedVal_);
 
     // Create a new block state and insert at it at the location of the new object.
     BlockState *state = BlockState::New(alloc_, obj_, undefinedVal_);
     startBlock_->insertAfter(obj_, state);
 
+    // Hold out of resume point until it is visited.
+    state->setInWorklist();
+
     *pState = state;
     return true;
 }
 
 void
 ObjectMemoryView::setEntryBlockState(BlockState *state)
 {
     state_ = state;
@@ -364,17 +341,17 @@ ObjectMemoryView::mergeIntoSuccessorStat
             succ->addPhi(phi);
             succState->setSlot(slot, phi);
         }
 
         // Insert the newly created block state instruction at the beginning
         // of the successor block, after all the phi nodes.  Note that it
         // would be captured by the entry resume point of the successor
         // block.
-        succ->insertBefore(*succ->begin(), succState);
+        succ->insertBefore(succ->safeInsertTop(), succState);
         *pSuccState = succState;
     }
 
     MOZ_ASSERT_IF(succ == startBlock_, startBlock_->isLoopHeader());
     if (succ->numPredecessors() > 1 && succState->numSlots() && succ != startBlock_) {
         // We need to re-compute successorWithPhis as the previous EliminatePhis
         // phase might have removed all the Phis from the successor block.
         size_t currIndex;
@@ -400,37 +377,48 @@ ObjectMemoryView::mergeIntoSuccessorStat
 }
 
 #ifdef DEBUG
 void
 ObjectMemoryView::assertSuccess()
 {
     for (MUseIterator i(obj_->usesBegin()); i != obj_->usesEnd(); i++) {
         MNode *ins = (*i)->consumer();
+        MDefinition *def = nullptr;
 
         // Resume points have been replaced by the object state.
-        MOZ_ASSERT(!ins->isResumePoint());
-
-        MDefinition *def = ins->toDefinition();
-
-        if (def->isRecoveredOnBailout())
+        if (ins->isResumePoint() || (def = ins->toDefinition())->isRecoveredOnBailout()) {
+            MOZ_ASSERT(obj_->isIncompleteObject());
             continue;
+        }
 
         // The only remaining uses would be removed by DCE, which will also
         // recover the object on bailouts.
         MOZ_ASSERT(def->isSlots() || def->isLambda());
         MOZ_ASSERT(!def->hasDefUses());
     }
 }
 #endif
 
 void
 ObjectMemoryView::visitResumePoint(MResumePoint *rp)
 {
-    ReplaceResumePointOperands(rp, obj_, state_);
+    // As long as the MObjectState is not yet seen next to the allocation, we do
+    // not patch the resume point to recover the side effects.
+    if (!state_->isInWorklist()) {
+        rp->addStore(alloc_, state_, lastResumePoint_);
+        lastResumePoint_ = rp;
+    }
+}
+
+void
+ObjectMemoryView::visitObjectState(MObjectState *ins)
+{
+    if (ins->isInWorklist())
+        ins->setNotInWorklist();
 }
 
 void
 ObjectMemoryView::visitStoreFixedSlot(MStoreFixedSlot *ins)
 {
     // Skip stores made on other objects.
     if (ins->object() != obj_)
         return;
@@ -520,16 +508,27 @@ ObjectMemoryView::visitFunctionEnvironme
 
     // Replace the function environment by the scope chain of the lambda.
     ins->replaceAllUsesWith(obj_);
 
     // Remove original instruction.
     ins->block()->discard(ins);
 }
 
+void
+ObjectMemoryView::visitLambda(MLambda *ins)
+{
+    if (ins->scopeChain() != obj_)
+        return;
+
+    // In order to recover the lambda we need to recover the scope chain, as the
+    // lambda is holding it.
+    ins->setIncompleteObject();
+}
+
 static bool
 IndexOf(MDefinition *ins, int32_t *res)
 {
     MOZ_ASSERT(ins->isLoadElement() || ins->isStoreElement());
     MDefinition *indexDef = ins->getOperand(1); // ins->index();
     if (indexDef->isBoundsCheck())
         indexDef = indexDef->toBoundsCheck()->index();
     if (indexDef->isToInt32())
@@ -707,16 +706,19 @@ class ArrayMemoryView : public MDefiniti
   private:
     TempAllocator &alloc_;
     MConstant *undefinedVal_;
     MConstant *length_;
     MInstruction *arr_;
     MBasicBlock *startBlock_;
     BlockState *state_;
 
+    // Used to improve the memory usage by sharing common modification.
+    const MResumePoint *lastResumePoint_;
+
   public:
     ArrayMemoryView(TempAllocator &alloc, MInstruction *arr);
 
     MBasicBlock *startingBlock();
     bool initStartingState(BlockState **pState);
 
     void setEntryBlockState(BlockState *state);
     bool mergeIntoSuccessorState(MBasicBlock *curr, MBasicBlock *succ, BlockState **pSuccState);
@@ -728,33 +730,38 @@ class ArrayMemoryView : public MDefiniti
 #endif
 
   private:
     bool isArrayStateElements(MDefinition *elements);
     void discardInstruction(MInstruction *ins, MDefinition *elements);
 
   public:
     void visitResumePoint(MResumePoint *rp);
+    void visitArrayState(MArrayState *ins);
     void visitStoreElement(MStoreElement *ins);
     void visitLoadElement(MLoadElement *ins);
     void visitSetInitializedLength(MSetInitializedLength *ins);
     void visitInitializedLength(MInitializedLength *ins);
     void visitArrayLength(MArrayLength *ins);
 };
 
 const char *ArrayMemoryView::phaseName = "Scalar Replacement of Array";
 
 ArrayMemoryView::ArrayMemoryView(TempAllocator &alloc, MInstruction *arr)
   : alloc_(alloc),
     undefinedVal_(nullptr),
     length_(nullptr),
     arr_(arr),
     startBlock_(arr->block()),
-    state_(nullptr)
+    state_(nullptr),
+    lastResumePoint_(nullptr)
 {
+    // Annotate snapshots RValue such that we recover the store first.
+    arr_->setIncompleteObject();
+
     // Annotate the instruction such that we do not replace it by a
     // Magic(JS_OPTIMIZED_OUT) in case of removed uses.
     arr_->setImplicitlyUsedUnchecked();
 }
 
 MBasicBlock *
 ArrayMemoryView::startingBlock()
 {
@@ -769,16 +776,19 @@ ArrayMemoryView::initStartingState(Block
     MConstant *initLength = MConstant::New(alloc_, Int32Value(0));
     arr_->block()->insertBefore(arr_, undefinedVal_);
     arr_->block()->insertBefore(arr_, initLength);
 
     // Create a new block state and insert at it at the location of the new array.
     BlockState *state = BlockState::New(alloc_, arr_, undefinedVal_, initLength);
     startBlock_->insertAfter(arr_, state);
 
+    // Hold out of resume point until it is visited.
+    state->setInWorklist();
+
     *pState = state;
     return true;
 }
 
 void
 ArrayMemoryView::setEntryBlockState(BlockState *state)
 {
     state_ = state;
@@ -831,17 +841,17 @@ ArrayMemoryView::mergeIntoSuccessorState
             succ->addPhi(phi);
             succState->setElement(index, phi);
         }
 
         // Insert the newly created block state instruction at the beginning
         // of the successor block, after all the phi nodes.  Note that it
         // would be captured by the entry resume point of the successor
         // block.
-        succ->insertBefore(*succ->begin(), succState);
+        succ->insertBefore(succ->safeInsertTop(), succState);
         *pSuccState = succState;
     }
 
     MOZ_ASSERT_IF(succ == startBlock_, startBlock_->isLoopHeader());
     if (succ->numPredecessors() > 1 && succState->numElements() && succ != startBlock_) {
         // We need to re-compute successorWithPhis as the previous EliminatePhis
         // phase might have removed all the Phis from the successor block.
         size_t currIndex;
@@ -872,17 +882,29 @@ ArrayMemoryView::assertSuccess()
 {
     MOZ_ASSERT(!arr_->hasLiveDefUses());
 }
 #endif
 
 void
 ArrayMemoryView::visitResumePoint(MResumePoint *rp)
 {
-    ReplaceResumePointOperands(rp, arr_, state_);
+    // As long as the MArrayState is not yet seen next to the allocation, we do
+    // not patch the resume point to recover the side effects.
+    if (!state_->isInWorklist()) {
+        rp->addStore(alloc_, state_, lastResumePoint_);
+        lastResumePoint_ = rp;
+    }
+}
+
+void
+ArrayMemoryView::visitArrayState(MArrayState *ins)
+{
+    if (ins->isInWorklist())
+        ins->setNotInWorklist();
 }
 
 bool
 ArrayMemoryView::isArrayStateElements(MDefinition *elements)
 {
     return elements->isElements() && elements->toElements()->object() == arr_;
 }