Backed out changeset 4ecf21c3c4a3 (bug 1042729)
authorEd Morley <emorley@mozilla.com>
Tue, 05 Aug 2014 13:33:27 +0100
changeset 197922 208cf26c5af831b732bd79848cefac698b4321cb
parent 197921 cf0b277b8255c88d5c43dd8d57511bf75f4f2636
child 197923 9f9247db062c2f53f188af3924db3e9c1486646a
push id27256
push userkwierso@gmail.com
push dateWed, 06 Aug 2014 00:06:20 +0000
treeherdermozilla-central@6cbdd4d523a7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1042729
milestone34.0a1
backs out4ecf21c3c4a3a2365526fc42a8035a5b421b0c05
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 4ecf21c3c4a3 (bug 1042729)
js/src/jit/IonAnalysis.cpp
js/src/jit/IonBuilder.cpp
js/src/jit/MIR.cpp
js/src/jit/MIR.h
js/src/jit/MIRGraph.cpp
js/src/jit/MIRGraph.h
js/src/jit/ParallelSafetyAnalysis.cpp
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -1739,25 +1739,20 @@ jit::AssertBasicGraphCoherency(MIRGraph 
         JS_ASSERT(&block->graph() == &graph);
 
         for (size_t i = 0; i < block->numSuccessors(); i++)
             JS_ASSERT(CheckSuccessorImpliesPredecessor(*block, block->getSuccessor(i)));
 
         for (size_t i = 0; i < block->numPredecessors(); i++)
             JS_ASSERT(CheckPredecessorImpliesSuccessor(*block, block->getPredecessor(i)));
 
-        MOZ_ASSERT_IF(block->entryResumePoint(), !block->entryResumePoint()->instruction());
         for (MResumePointIterator iter(block->resumePointsBegin()); iter != block->resumePointsEnd(); iter++) {
-            // We cannot yet assert that is there is no instruction then this is
-            // the entry resume point because we are still storing resume points
-            // in the InlinePropertyTable.
-            MOZ_ASSERT_IF(iter->instruction(), iter->instruction()->block() == *block);
             for (uint32_t i = 0, e = iter->numOperands(); i < e; i++) {
-                MOZ_ASSERT(iter->getUseFor(i)->hasProducer());
-                MOZ_ASSERT(CheckOperandImpliesUse(*iter, iter->getOperand(i)));
+                if (iter->getUseFor(i)->hasProducer())
+                    JS_ASSERT(CheckOperandImpliesUse(*iter, iter->getOperand(i)));
             }
         }
         for (MPhiIterator phi(block->phisBegin()); phi != block->phisEnd(); phi++) {
             JS_ASSERT(phi->numOperands() == block->numPredecessors());
             MOZ_ASSERT(!phi->isRecoveredOnBailout());
         }
         for (MDefinitionIterator iter(*block); iter; iter++) {
             JS_ASSERT(iter->block() == *block);
@@ -1765,18 +1760,21 @@ jit::AssertBasicGraphCoherency(MIRGraph 
             // Assert that use chains are valid for this instruction.
             for (uint32_t i = 0, end = iter->numOperands(); i < end; i++)
                 JS_ASSERT(CheckOperandImpliesUse(*iter, iter->getOperand(i)));
             for (MUseIterator use(iter->usesBegin()); use != iter->usesEnd(); use++)
                 JS_ASSERT(CheckUseImpliesOperand(*iter, *use));
 
             if (iter->isInstruction()) {
                 if (MResumePoint *resume = iter->toInstruction()->resumePoint()) {
-                    MOZ_ASSERT(resume->instruction() == *iter);
-                    MOZ_ASSERT(resume->block() == *block);
+                    if (MInstruction *ins = resume->instruction())
+                        JS_ASSERT(ins->block() == iter->block());
+
+                    for (uint32_t i = 0, e = resume->numOperands(); i < e; i++)
+                        MOZ_ASSERT(resume->getUseFor(i)->hasProducer());
                 }
             }
 
             if (iter->isRecoveredOnBailout())
                 MOZ_ASSERT(!iter->hasLiveDefUses());
         }
     }
 
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -6046,16 +6046,17 @@ IonBuilder::resume(MInstruction *ins, js
 {
     JS_ASSERT(ins->isEffectful() || !ins->isMovable());
 
     MResumePoint *resumePoint = MResumePoint::New(alloc(), ins->block(), pc, callerResumePoint_,
                                                   mode);
     if (!resumePoint)
         return false;
     ins->setResumePoint(resumePoint);
+    resumePoint->setInstruction(ins);
     return true;
 }
 
 bool
 IonBuilder::resumeAt(MInstruction *ins, jsbytecode *pc)
 {
     return resume(ins, pc, MResumePoint::ResumeAt);
 }
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -226,33 +226,16 @@ MDefinition::analyzeEdgeCasesForward()
 {
 }
 
 void
 MDefinition::analyzeEdgeCasesBackward()
 {
 }
 
-void
-MInstruction::setResumePoint(MResumePoint *resumePoint)
-{
-    JS_ASSERT(!resumePoint_);
-    resumePoint_ = resumePoint;
-    resumePoint_->setInstruction(this);
-}
-
-void
-MInstruction::stealResumePoint(MInstruction *ins)
-{
-    MOZ_ASSERT(ins->resumePoint_->instruction() == ins);
-    resumePoint_ = ins->resumePoint_;
-    ins->resumePoint_ = nullptr;
-    resumePoint_->replaceInstruction(this);
-}
-
 static bool
 MaybeEmulatesUndefined(MDefinition *op)
 {
     if (!op->mightBeType(MIRType_Object))
         return false;
 
     types::TemporaryTypeSet *types = op->resultTypeSet();
     if (!types)
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -774,19 +774,25 @@ class MInstruction
 {
     MResumePoint *resumePoint_;
 
   public:
     MInstruction()
       : resumePoint_(nullptr)
     { }
 
-    void setResumePoint(MResumePoint *resumePoint);
+    void setResumePoint(MResumePoint *resumePoint) {
+        JS_ASSERT(!resumePoint_);
+        resumePoint_ = resumePoint;
+    }
     // Used to transfer the resume point to the rewritten instruction.
-    void stealResumePoint(MInstruction *ins);
+    void stealResumePoint(MInstruction *ins) {
+        resumePoint_ = ins->resumePoint_;
+        ins->resumePoint_ = nullptr;
+    }
     MResumePoint *resumePoint() const {
         return resumePoint_;
     }
 };
 
 #define INSTRUCTION_HEADER(opcode)                                          \
     Opcode op() const {                                                     \
         return MDefinition::Op_##opcode;                                    \
@@ -10291,22 +10297,16 @@ class MResumePoint MOZ_FINAL : public MN
         for (MResumePoint *it = caller_; it; it = it->caller_)
             count++;
         return count;
     }
     MInstruction *instruction() {
         return instruction_;
     }
     void setInstruction(MInstruction *ins) {
-        MOZ_ASSERT(!instruction_);
-        instruction_ = ins;
-    }
-    // Only to be used by stealResumePoint.
-    void replaceInstruction(MInstruction *ins) {
-        MOZ_ASSERT(instruction_);
         instruction_ = ins;
     }
     Mode mode() const {
         return mode_;
     }
 
     void discardUses() {
         for (size_t i = 0; i < operands_.length(); i++) {
--- a/js/src/jit/MIRGraph.cpp
+++ b/js/src/jit/MIRGraph.cpp
@@ -443,19 +443,16 @@ MBasicBlock::inheritSlots(MBasicBlock *p
 {
     stackPosition_ = parent->stackPosition_;
     copySlots(parent);
 }
 
 bool
 MBasicBlock::initEntrySlots(TempAllocator &alloc)
 {
-    // Remove the previous resume point.
-    discardResumePoint(entryResumePoint_);
-
     // Create a resume point using our initial stack state.
     entryResumePoint_ = MResumePoint::New(alloc, this, pc(), callerResumePoint(),
                                           MResumePoint::ResumeAt);
     if (!entryResumePoint_)
         return false;
     return true;
 }
 
@@ -715,38 +712,36 @@ AssertSafelyDiscardable(MDefinition *def
 #ifdef DEBUG
     // Instructions captured by resume points cannot be safely discarded, since
     // they are necessary for interpreter frame reconstruction in case of bailout.
     JS_ASSERT(!def->hasUses());
 #endif
 }
 
 void
-MBasicBlock::discardResumePoint(MResumePoint *rp)
-{
-    rp->discardUses();
-    MResumePointIterator iter = resumePointsBegin();
-    while (*iter != rp) {
-        // We should reach it before reaching the end.
-        MOZ_ASSERT(iter != resumePointsEnd());
-        iter++;
-    }
-    resumePoints_.removeAt(iter);
-}
-
-void
 MBasicBlock::prepareForDiscard(MInstruction *ins, ReferencesType refType /* = RefType_Default */)
 {
     // Only remove instructions from the same basic block.  This is needed for
     // correctly removing the resume point if any.
     MOZ_ASSERT(ins->block() == this);
 
     MResumePoint *rp = ins->resumePoint();
-    if (refType & RefType_DiscardResumePoint && rp)
-        discardResumePoint(rp);
+    if (refType & RefType_DiscardResumePoint && rp) {
+        rp->discardUses();
+        // Resume point are using a forward list only, so we need to iterate
+        // to the location of the resume point in order to remove it.
+        MResumePointIterator iter = resumePointsBegin();
+        while (*iter != rp) {
+            // If the instruction has a resume point, then it should be part
+            // of the basic block list of resume points.
+            MOZ_ASSERT(iter != resumePointsEnd());
+            iter++;
+        }
+        resumePoints_.removeAt(iter);
+    }
 
     // We need to assert that instructions have no uses after removing the their
     // resume points operands as they could be captured by their own resume
     // point.
     MOZ_ASSERT_IF(refType & RefType_AssertNoUses, !ins->hasUses());
 
     MOZ_ASSERT(refType & RefType_DiscardOperands);
     for (size_t i = 0, e = ins->numOperands(); i < e; i++)
--- a/js/src/jit/MIRGraph.h
+++ b/js/src/jit/MIRGraph.h
@@ -56,18 +56,16 @@ class MBasicBlock : public TempObject, p
 
     // Pushes a copy of a local variable or argument.
     void pushVariable(uint32_t slot);
 
     // Sets a variable slot to the top of the stack, correctly creating copies
     // as needed.
     void setVariable(uint32_t slot);
 
-    void discardResumePoint(MResumePoint *rp);
-
     enum ReferencesType {
         RefType_AssertNoUses = 1 << 0,
         RefType_DiscardOperands = 1 << 1,
         RefType_DiscardResumePoint = 1 << 2,
         RefType_Default = RefType_AssertNoUses | RefType_DiscardOperands | RefType_DiscardResumePoint
     };
 
     // Remove all references to an instruction such that it can be removed from
--- a/js/src/jit/ParallelSafetyAnalysis.cpp
+++ b/js/src/jit/ParallelSafetyAnalysis.cpp
@@ -332,18 +332,21 @@ class ParallelSafetyVisitor : public MDe
     // It looks like this could easily be made safe:
     UNSAFE_OP(ConvertElementsToDoubles)
 };
 
 static void
 TransplantResumePoint(MInstruction *oldInstruction, MInstruction *replacementInstruction)
 {
     MOZ_ASSERT(!oldInstruction->isDiscarded());
-    if (oldInstruction->resumePoint())
+    if (MResumePoint *rp = oldInstruction->resumePoint()) {
         replacementInstruction->stealResumePoint(oldInstruction);
+        if (rp->instruction() == oldInstruction)
+            rp->setInstruction(replacementInstruction);
+    }
 }
 
 bool
 ParallelSafetyAnalysis::analyze()
 {
     // Walk the basic blocks in a DFS.  When we encounter a block with an
     // unsafe instruction, then we know that this block will bailout when
     // executed.  Therefore, we replace the block.