[webkit-changes] [WebKit/WebKit] 8495ff: PutStack sinking phase inserts PutStack with wrong...

Commit Queue noreply at github.com
Fri Jul 7 14:42:04 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 8495ff2f3399f19ca2f88fa53c77ab00d83ce692
      https://github.com/WebKit/WebKit/commit/8495ff2f3399f19ca2f88fa53c77ab00d83ce692
  Author: David Degazio <d_degazio at apple.com>
  Date:   2023-07-07 (Fri, 07 Jul 2023)

  Changed paths:
    A JSTests/stress/PutStack-sinking-through-DeadFlush-unification.js
    M Source/JavaScriptCore/dfg/DFGPlan.cpp
    M Source/JavaScriptCore/dfg/DFGPutStackSinkingPhase.cpp
    M Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp

  Log Message:
  -----------
  PutStack sinking phase inserts PutStack with wrong value
https://bugs.webkit.org/show_bug.cgi?id=256872
rdar://109752832

Reviewed by Keith Miller.

Fixes a bug where the DFG PutStack sinking phase would put the wrong
value to the stack. This is due to the PutStack sinking phase doing its
own SSA calculation to determine the correct locations to insert
PutStacks, which is not necessarily the same as correctly tracking the
values that should belong in each PutStack (which is just SSA conversion).

This patch adds an additional PutStack node after each Phi inserted in
SSA conversion. As a result, when PutStack sinking interprets each block
to map each variable to its current value, it correctly associates
existing Phis with their variables, even if the PutStack sinking phase's
SSA calculation would not insert a Phi there.

One concern with this patch is performance - specifically, adding
additional PutStacks might pessimize arguments elimination. To address
this, this patch runs PutStack sinking both before and after arguments
elimination, so we reduce spurious PutStacking before eliminating
arguments, and re-run it after to exploit any opportunities created by
the existing phase ordering. Testing on both Speedometer 2 and JetStream
2, there is no significant change in score with this patch.

* JSTests/stress/PutStack-sinking-through-DeadFlush-unification.js: Added.
(test):
(main):
* Source/JavaScriptCore/dfg/DFGPlan.cpp:
(JSC::DFG::Plan::compileInThreadImpl):
* Source/JavaScriptCore/dfg/DFGPutStackSinkingPhase.cpp:
* Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp:
(JSC::DFG::SSAConversionPhase::run):

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




More information about the webkit-changes mailing list