Bug 1077720 - IonMonkey: Optimize MPhi::removeOperand r=nbp
authorDan Gohman <sunfish@mozilla.com>
Wed, 08 Oct 2014 15:04:11 -0700
changeset 209488 c4a22f350c13ca4f2607b3873f80e4ede17f791c
parent 209487 1cc7fa64a589453961df1aacf6d07a7761a3897d
child 209489 49aafe51147f82cfadccdfe86ff761f2f4e9bbc0
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersnbp
bugs1077720
milestone35.0a1
Bug 1077720 - IonMonkey: Optimize MPhi::removeOperand r=nbp
js/src/jit/InlineList.h
js/src/jit/MIR.cpp
js/src/jit/MIR.h
--- a/js/src/jit/InlineList.h
+++ b/js/src/jit/InlineList.h
@@ -332,16 +332,28 @@ class InlineList : protected InlineListN
     void remove(Node *t) {
         Node *tNext = static_cast<Node *>(t->next);
         Node *tPrev = t->prev;
         tPrev->next = tNext;
         tNext->prev = tPrev;
         t->next = nullptr;
         t->prev = nullptr;
     }
+    // Remove |old| from the list and insert |now| in its place.
+    void replace(Node *old, Node *now) {
+        MOZ_ASSERT(now->next == nullptr && now->prev == nullptr);
+        Node *listNext = static_cast<Node *>(old->next);
+        Node *listPrev = old->prev;
+        listPrev->next = now;
+        listNext->prev = now;
+        now->next = listNext;
+        now->prev = listPrev;
+        old->next = nullptr;
+        old->prev = nullptr;
+    }
     void clear() {
         this->next = this->prev = this;
     }
     bool empty() const {
         return begin() == end();
     }
     void takeElements(InlineList &l) {
         MOZ_ASSERT(&l != this, "cannot takeElements from this");
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -1187,30 +1187,34 @@ MPhi::removeOperand(size_t index)
 {
     MOZ_ASSERT(index < numOperands());
     MOZ_ASSERT(getUseFor(index)->index() == index);
     MOZ_ASSERT(getUseFor(index)->consumer() == this);
 
     // If we have phi(..., a, b, c, d, ..., z) and we plan
     // on removing a, then first shift downward so that we have
     // phi(..., b, c, d, ..., z, z):
-    size_t length = inputs_.length();
-    for (size_t i = index; i < length - 1; i++)
-        inputs_[i].replaceProducer(inputs_[i + 1].producer());
+    MUse *p = inputs_.begin() + index;
+    MUse *e = inputs_.end();
+    p->producer()->removeUse(p);
+    for (; p < e - 1; ++p) {
+        MDefinition *producer = (p + 1)->producer();
+        p->setProducerUnchecked(producer);
+        producer->replaceUse(p + 1, p);
+    }
 
     // truncate the inputs_ list:
-    inputs_[length - 1].releaseProducer();
-    inputs_.shrinkBy(1);
+    inputs_.popBack();
 }
 
 void
 MPhi::removeAllOperands()
 {
-    for (size_t i = 0; i < inputs_.length(); i++)
-        inputs_[i].releaseProducer();
+    for (MUse *p = inputs_.begin(), *e = inputs_.end(); p < e; ++p)
+        p->producer()->removeUse(p);
     inputs_.clear();
 }
 
 MDefinition *
 MPhi::foldsTernary()
 {
     /* Look if this MPhi is a ternary construct.
      * This is a very loose term as it actually only checks for
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -107,35 +107,39 @@ MIRType MIRTypeFromValue(const js::Value
      */                                                                         \
     _(Discarded)
 
 class MDefinition;
 class MInstruction;
 class MBasicBlock;
 class MNode;
 class MUse;
+class MPhi;
 class MIRGraph;
 class MResumePoint;
 class MControlInstruction;
 
 // Represents a use of a node.
 class MUse : public TempObject, public InlineListNode<MUse>
 {
+    // Grant access to setProducerUnchecked.
     friend class MDefinition;
+    friend class MPhi;
 
     MDefinition *producer_; // MDefinition that is being used.
     MNode *consumer_;       // The node that is using this operand.
 
     MUse(MDefinition *producer, MNode *consumer)
       : producer_(producer),
         consumer_(consumer)
     { }
 
-    // Low-level unchecked edit method for replaceAllUsesWith. This doesn't
-    // update use lists! replaceAllUsesWith does that manually.
+    // Low-level unchecked edit method for replaceAllUsesWith and
+    // MPhi::removeOperand. This doesn't update use lists!
+    // replaceAllUsesWith and MPhi::removeOperand do that manually.
     void setProducerUnchecked(MDefinition *producer) {
         MOZ_ASSERT(consumer_);
         MOZ_ASSERT(producer_);
         MOZ_ASSERT(producer);
         producer_ = producer;
     }
 
   public:
@@ -658,21 +662,27 @@ class MDefinition : public MNode
     // (only counting MDefinitions, ignoring MResumePoints)
     bool hasLiveDefUses() const;
 
     bool hasUses() const {
         return !uses_.empty();
     }
 
     void addUse(MUse *use) {
+        MOZ_ASSERT(use->producer() == this);
         uses_.pushFront(use);
     }
     void addUseUnchecked(MUse *use) {
+        MOZ_ASSERT(use->producer() == this);
         uses_.pushFrontUnchecked(use);
     }
+    void replaceUse(MUse *old, MUse *now) {
+        MOZ_ASSERT(now->producer() == this);
+        uses_.replace(old, now);
+    }
     void replaceAllUsesWith(MDefinition *dom);
 
     // Like replaceAllUsesWith, but doesn't set UseRemoved on |this|'s operands.
     void justReplaceAllUsesWith(MDefinition *dom);
 
     // Mark this instruction as having replaced all uses of ins, as during GVN,
     // returning false if the replacement should not be performed. For use when
     // GVN eliminates instructions which are not equivalent to one another.