Bug 727480 - Guarantee all operands have set uses. r=dvander
authorSean Stangl <sstangl@mozilla.com>
Fri, 24 Feb 2012 15:35:14 -0800
changeset 105934 ebd61b600176d9fe2cfef187ae028515ee4f722a
parent 105933 1a9e91a88a5488d15b39a0530c326fdb1e1ce77b
child 105935 418840ca313e0f8a3574833a3e33c7f67e004f47
push id14706
push usereakhgari@mozilla.com
push dateTue, 11 Sep 2012 20:39:52 +0000
treeherdermozilla-inbound@d50bf1edaabe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdvander
bugs727480
milestone13.0a1
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
Bug 727480 - Guarantee all operands have set uses. r=dvander
js/src/ion/Ion.cpp
js/src/ion/IonAnalysis.cpp
js/src/ion/IonBuilder.cpp
js/src/ion/LICM.cpp
js/src/ion/MCallOptimize.cpp
js/src/ion/MIR.h
js/src/ion/MIRGraph.cpp
js/src/ion/MIRGraph.h
--- a/js/src/ion/Ion.cpp
+++ b/js/src/ion/Ion.cpp
@@ -658,54 +658,61 @@ TestCompiler(IonBuilder &builder, MIRGra
     if (!BuildDominatorTree(graph))
         return false;
     // No spew: graph not changed.
 
     // This must occur before any code elimination.
     if (!EliminateDeadPhis(graph))
         return false;
     IonSpewPass("Eliminate dead phis");
+    AssertGraphCoherency(graph);
 
     if (!BuildPhiReverseMapping(graph))
         return false;
     // No spew: graph not changed.
 
     EliminateCopies(graph);
     IonSpewPass("Eliminate copies");
+    AssertGraphCoherency(graph);
 
     // This pass also removes copies.
     if (!ApplyTypeInformation(graph))
         return false;
     IonSpewPass("Apply types");
+    AssertGraphCoherency(graph);
 
     // Alias analysis is required for LICM and GVN so that we don't move
     // loads across stores.
     if (js_IonOptions.licm || js_IonOptions.gvn) {
         AliasAnalysis analysis(graph);
         if (!analysis.analyze())
             return false;
         IonSpewPass("Alias analysis");
+        AssertGraphCoherency(graph);
     }
 
     if (js_IonOptions.gvn) {
         ValueNumberer gvn(graph, js_IonOptions.gvnIsOptimistic);
         if (!gvn.analyze())
             return false;
         IonSpewPass("GVN");
+        AssertGraphCoherency(graph);
     }
 
     if (!EliminateDeadCode(graph))
         return false;
     IonSpewPass("DCE");
+    AssertGraphCoherency(graph);
 
     if (js_IonOptions.licm) {
         LICM licm(graph);
         if (!licm.analyze())
             return false;
         IonSpewPass("LICM");
+        AssertGraphCoherency(graph);
     }
 
     LIRGraph lir(graph);
     LIRGenerator lirgen(&builder, graph, lir);
     if (!lirgen.generate())
         return false;
     IonSpewPass("Generate LIR");
 
--- a/js/src/ion/IonAnalysis.cpp
+++ b/js/src/ion/IonAnalysis.cpp
@@ -732,43 +732,64 @@ ion::FindNaturalLoops(MIRGraph &graph)
                 break;
             current = worklist.popCopy();
         } while (true);
     }
 
     return true;
 }
 
+#ifdef DEBUG
+static bool
+CheckSuccessorImpliesPredecessor(MBasicBlock *A, MBasicBlock *B)
+{
+    // Assuming B = succ(A), verify A = pred(B).
+    for (size_t i = 0; i < B->numPredecessors(); i++) {
+        if (A == B->getPredecessor(i))
+            return true;
+    }
+    return false;
+}
+
+static bool
+CheckPredecessorImpliesSuccessor(MBasicBlock *A, MBasicBlock *B)
+{
+    // Assuming B = pred(A), verify A = succ(B).
+    for (size_t i = 0; i < B->numSuccessors(); i++) {
+        if (A == B->getSuccessor(i))
+            return true;
+    }
+    return false;
+}
+
+static bool
+CheckMarkedAsUse(MInstruction *ins, MDefinition *operand)
+{
+    for (MUseIterator i = operand->usesBegin(); i != operand->usesEnd(); i++) {
+        if (i->node()->isDefinition()) {
+            if (ins == i->node()->toDefinition());
+                return true;
+        }
+    }
+    return false;
+}
+#endif // DEBUG
+
 void
 ion::AssertGraphCoherency(MIRGraph &graph)
 {
 #ifdef DEBUG
     // Assert successor and predecessor list coherency.
     for (MBasicBlockIterator block(graph.begin()); block != graph.end(); block++) {
-        // B = succ(A) must imply A = pred(B).
-        for (size_t i = 0; i < block->numSuccessors(); i++) {
-            MBasicBlock *succ = block->getSuccessor(i);
-            int found = 0;
-
-            for (size_t j = 0; j < succ->numPredecessors(); j++) {
-                if (succ->getPredecessor(j) == *block)
-                    found++;
-            }
+        for (size_t i = 0; i < block->numSuccessors(); i++)
+            JS_ASSERT(CheckSuccessorImpliesPredecessor(*block, block->getSuccessor(i)));
 
-            JS_ASSERT(found == 1);
-        }
+        for (size_t i = 0; i < block->numPredecessors(); i++)
+            JS_ASSERT(CheckPredecessorImpliesSuccessor(*block, block->getPredecessor(i)));
 
-        // A = pred(B) must imply B = succ(A).
-        for (size_t i = 0; i < block->numPredecessors(); i++) {
-            MBasicBlock *pred = block->getPredecessor(i);
-            int found = 0;
-
-            for (size_t j = 0; j < pred->numSuccessors(); j++) {
-                if (pred->getSuccessor(j) == *block)
-                    found++;
-            }
-
-            JS_ASSERT(found == 1);
+        for (MInstructionIterator ins = block->begin(); ins != block->end(); ins++) {
+            for (uint32 i = 0; i < ins->numOperands(); i++)
+                JS_ASSERT(CheckMarkedAsUse(*ins, ins->getOperand(i)));
         }
     }
 #endif
 }
 
--- a/js/src/ion/IonBuilder.cpp
+++ b/js/src/ion/IonBuilder.cpp
@@ -2480,17 +2480,17 @@ IonBuilder::jsop_call(uint32 argc, bool 
         MPassArg *oldarg = current->pop()->toPassArg();
 
         // Supply a special constructing Magic value.
         MConstant *magic = MConstant::New(MagicValue(JS_IS_CONSTRUCTING));
         oldarg->block()->insertBefore(oldarg, magic);
         MPassArg *newthis = MPassArg::New(magic);
         oldarg->block()->insertBefore(oldarg, newthis);
 
-        oldarg->block()->remove(oldarg);
+        oldarg->block()->discard(oldarg);
         current->push(newthis);
     }
 
     // Pass |this| and function.
     call->addArg(0, current->pop()->toPassArg());
     call->initFunction(current->pop());
 
     if (target)
--- a/js/src/ion/LICM.cpp
+++ b/js/src/ion/LICM.cpp
@@ -345,18 +345,17 @@ Loop::hoistInstructions(InstructionQueue
 
         // Loads may have an implicit dependency on either stores (effectful instructions) or
         // control instructions so we should never move these.
         JS_ASSERT(!ins->isControlInstruction());
         JS_ASSERT(!ins->isEffectful());
         JS_ASSERT(ins->isMovable());
 
         if (checkHotness(ins->block())) {
-            ins->block()->remove(ins);
-            preLoop_->insertBefore(preLoop_->lastIns(), ins);
+            ins->block()->moveBefore(preLoop_->lastIns(), ins);
             ins->setNotLoopInvariant();
         }
     }
 
     for (size_t i = 0; i < hoistedChecks.length(); i++) {
         MInstruction *ins = hoistedChecks[i];
         preLoop_->insertBefore(preLoop_->lastIns(), ins);
     }
--- a/js/src/ion/MCallOptimize.cpp
+++ b/js/src/ion/MCallOptimize.cpp
@@ -55,17 +55,17 @@ IonBuilder::discardCallArgs(uint32 argc,
 
     // Bytecode order: Function, This, Arg0, Arg1, ..., ArgN, Call.
     // Copy PassArg arguments from ArgN to This.
     for (int32 i = argc; i >= 0; i--) {
         MPassArg *passArg = bb->pop()->toPassArg();
         MBasicBlock *block = passArg->block();
         MDefinition *wrapped = passArg->getArgument();
         passArg->replaceAllUsesWith(wrapped);
-        block->remove(passArg);
+        block->discard(passArg);
         argv[i] = wrapped;
     }
 
     return true;
 }
 
 bool
 IonBuilder::discardCall(uint32 argc, MDefinitionVector &argv, MBasicBlock *bb)
--- a/js/src/ion/MIR.h
+++ b/js/src/ion/MIR.h
@@ -2122,25 +2122,25 @@ class MPhi : public MDefinition, public 
 
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
 };
 
 // MIR representation of a Value on the OSR StackFrame.
 // The Value is indexed off of OsrFrameReg.
-class MOsrValue : public MAryInstruction<1>
+class MOsrValue : public MUnaryInstruction
 {
   private:
     ptrdiff_t frameOffset_;
 
     MOsrValue(MOsrEntry *entry, ptrdiff_t frameOffset)
-      : frameOffset_(frameOffset)
+      : MUnaryInstruction(entry),
+        frameOffset_(frameOffset)
     {
-        setOperand(0, entry);
         setResultType(MIRType_Value);
     }
 
   public:
     INSTRUCTION_HEADER(OsrValue);
     static MOsrValue *New(MOsrEntry *entry, ptrdiff_t frameOffset) {
         return new MOsrValue(entry, frameOffset);
     }
@@ -2155,22 +2155,22 @@ class MOsrValue : public MAryInstruction
 
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
 };
 
 // MIR representation of a JSObject scope chain pointer on the OSR StackFrame.
 // The pointer is indexed off of OsrFrameReg.
-class MOsrScopeChain : public MAryInstruction<1>
+class MOsrScopeChain : public MUnaryInstruction
 {
   private:
     MOsrScopeChain(MOsrEntry *entry)
+      : MUnaryInstruction(entry)
     {
-        setOperand(0, entry);
         setResultType(MIRType_Object);
     }
 
   public:
     INSTRUCTION_HEADER(OsrScopeChain);
     static MOsrScopeChain *New(MOsrEntry *entry) {
         return new MOsrScopeChain(entry);
     }
@@ -2536,18 +2536,18 @@ class MInitializedLength
 };
 
 // Set a dense array's initialized length to an elements vector.
 class MSetInitializedLength
   : public MAryInstruction<2>
 {
     MSetInitializedLength(MDefinition *elements, MDefinition *index)
     {
-        setOperand(0, elements);
-        setOperand(1, index);
+        initOperand(0, elements);
+        initOperand(1, index);
     }
 
   public:
     INSTRUCTION_HEADER(SetInitializedLength);
 
     static MSetInitializedLength *New(MDefinition *elements, MDefinition *index) {
         return new MSetInitializedLength(elements, index);
     }
@@ -3752,16 +3752,17 @@ class MResumePoint : public MNode
     MResumePoint *caller_;
     Mode mode_;
 
     MResumePoint(MBasicBlock *block, jsbytecode *pc, MResumePoint *parent, Mode mode);
     bool init(MBasicBlock *state);
     void inherit(MBasicBlock *state);
 
   protected:
+    // Overwrites an operand without updating its Uses.
     void setOperand(size_t index, MDefinition *operand) {
         JS_ASSERT(index < stackDepth_);
         operands_[index] = operand;
     }
 
   public:
     static MResumePoint *New(MBasicBlock *block, jsbytecode *pc, MResumePoint *parent, Mode mode);
 
--- a/js/src/ion/MIRGraph.cpp
+++ b/js/src/ion/MIRGraph.cpp
@@ -531,36 +531,42 @@ MDefinition *
 MBasicBlock::peek(int32 depth)
 {
     JS_ASSERT(depth < 0);
     JS_ASSERT(stackPosition_ + depth >= info_.firstStackSlot());
     return getSlot(stackPosition_ + depth);
 }
 
 void
-MBasicBlock::remove(MInstruction *ins)
-{
-    for (size_t i = 0; i < ins->numOperands(); i++)
-        ins->replaceOperand(i, NULL);
-
-    instructions_.remove(ins);
-}
-
-void
 MBasicBlock::discardLastIns()
 {
     JS_ASSERT(lastIns_);
     discard(lastIns_);
     lastIns_ = NULL;
 }
 
 void
+MBasicBlock::moveBefore(MInstruction *at, MInstruction *ins)
+{
+    // Remove |ins| from the current block.
+    JS_ASSERT(ins->block() == this);
+    instructions_.remove(ins);
+
+    // Insert into new block, which may be distinct.
+    // Uses and operands are untouched.
+    at->block()->insertBefore(at, ins);
+}
+
+void
 MBasicBlock::discard(MInstruction *ins)
 {
-    remove(ins);
+    for (size_t i = 0; i < ins->numOperands(); i++)
+        ins->replaceOperand(i, NULL);
+
+    instructions_.remove(ins);
 }
 
 MInstructionIterator
 MBasicBlock::discardAt(MInstructionIterator &iter)
 {
     for (size_t i = 0; i < iter->numOperands(); i++)
         iter->replaceOperand(i, NULL);
 
--- a/js/src/ion/MIRGraph.h
+++ b/js/src/ion/MIRGraph.h
@@ -208,19 +208,18 @@ class MBasicBlock : public TempObject, p
     bool setBackedge(MBasicBlock *block);
 
     // Propagates phis placed in a loop header down to this successor block.
     void inheritPhis(MBasicBlock *header);
 
     void insertBefore(MInstruction *at, MInstruction *ins);
     void insertAfter(MInstruction *at, MInstruction *ins);
 
-    // Removes an instruction from the instruction list with the intention to
-    // re-insert it at a different location.
-    void remove(MInstruction *ins);
+    // Move an instruction. Movement may cross block boundaries.
+    void moveBefore(MInstruction *at, MInstruction *ins);
 
     // Removes an instruction with the intention to discard it.
     void discard(MInstruction *ins);
     void discardLastIns();
     MInstructionIterator discardAt(MInstructionIterator &iter);
     MInstructionReverseIterator discardAt(MInstructionReverseIterator &iter);
     MDefinitionIterator discardDefAt(MDefinitionIterator &iter);