[webkit-changes] [WebKit/WebKit] ba4442: [JSC] ObjectAllocationSinking should not omit phi ...

[Yusuke Suzuki]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: ba44420c913e8089e187ae42b9fe596f0332fd05
      https://github.com/WebKit/WebKit/commit/ba44420c913e8089e187ae42b9fe596f0332fd05
  Author: Yusuke Suzuki <ysuzuki at apple.com>
  Date:   2024-09-12 (Thu, 12 Sep 2024)

  Changed paths:
    A JSTests/stress/object-allocation-sinking-phi-insertion-for-pointers.js
    M Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp

  Log Message:
  -----------
  [JSC] ObjectAllocationSinking should not omit phi insertion when pointer 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



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications