[JSC] ObjectAllocationSinking should not omit phi insertion when poin…

…ter follows to the same value

https://bugs.webkit.org/show_bug.cgi?id=279570
rdar://135851156

Reviewed by Keith Miller.

Let's consider the following FTL graph.

    BB#0
    @0 = NewObject()
    Jump #1

    BB#1
    PutByOffset(@0, 0, @x)
    Jump #2

    BB#2
    ...
    @z = ...
    @1 = GetByOffset(@x, 0)
    Branch(@1, #3, #4)

    BB#3
    PutByOffset(@0, 0, @z)
    Jump #5

    BB#4
    PutByOffset(@0, 0, @z)
    Jump #5

    BB#5
    Jump #2

Now, we would like to eliminate @0 object allocation. And we are
computing SSA for pointers of fields of the that object which gets
eliminated. Consider about @x's fields' SSA. PutByOffset becomes Def
and GetByOffset becomes Use. And the same field will get the same SSA
variable. So we first puts Defs and compute Phis based on that.

In ObjectAllocationSinking phase, we had a fast path when the both SSA
variable is following to the same value. Let's see BB#5. Because BB#3
and BB#4 defines Defs, dominance frontier BB#5 will need to introduce
Phi. But interestingly, both SSA variable is following to the same @z.
As a result, we were not inserting Phi for this case.

But this is wrong. Inserted Phi is a Def, and based on that, we will
further introduce Phis with that. If we omit inserting Phi in BB#5,
we will not insert Phi into BB#2 while BB#2 will merge BB#1's Def And
BB#5's Phi's Def. As a result, in BB#2, we think this variable is
following to BB#1's Def. But that's wrong and BB#5's Phi exists.

This patch removes this fast path to fix the issue.

* JSTests/stress/object-allocation-sinking-phi-insertion-for-pointers.js: Added.
(Queue):
(Queue.prototype.enqueue):
(Queue.prototype.dequeue):
(i.queue.dequeue):
* Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:

Canonical link: https://commits.webkit.org/283558@main

JSTests/stress/object-allocation-sinking-phi-insertion-for-pointers.js

@@ -0,0 +1,54 @@
+class Queue {
+  _head;
+  _tail;
+  _length;
+  constructor(items) {
+    this._head = null;
+    this._tail = null;
+    this._length = 0;
+    if (items) {
+      for (const item of items) {
+        this.enqueue(item);
+      }
+    }
+  }
+  enqueue(item) {
+    const entry = {
+      next: null,
+      value: item,
+    };
+    if (this._tail) {
+      this._tail.next = entry;
+      this._tail = entry;
+    } else {
+      this._head = entry;
+      this._tail = entry;
+    }
+    this._length++;
+  }
+  dequeue() {
+    const entry = this._head;
+    if (entry) {
+      this._head = entry.next;
+      this._length--;
+      if (this._head === null) {
+        this._tail = null;
+      }
+      return entry.value;
+    } else {
+      return null;
+    }
+  }
+}
+for (let i = 0; i < 1e5; i++) {
+  const queue = new Queue(new Set(["foo", "bar", "baz"]));
+  if (queue.dequeue() !== "foo") {
+    throw new Error("Expected foo");
+  }
+  if (queue.dequeue() !== "bar") {
+    throw new Error("Expected bar");
+  }
+  if (queue.dequeue() !== "baz") {
+    throw new Error("Expected baz");
+  }
+}

Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp

@@ -1909,11 +1909,6 @@ class ObjectAllocationSinkingPhase : public Phase {
                 if (m_heapAtHead[block].getAllocation(location.base()).isEscapedAllocation())
                     return nullptr;
 
-                // If we point to a single allocation, we will
-                // directly use its materialization
-                if (m_heapAtHead[block].follow(location))
-                    return nullptr;
-
                 Node* phiNode = m_graph.addNode(SpecHeapTop, Phi, block->at(0)->origin.withInvalidExit());
                 phiNode->mergeFlags(NodeResultJS);
                 return phiNode;