[webkit-changes] [WebKit/WebKit] f5b95a: [JSC] We should fix the backwards propagation for ...

[Commit Queue]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: f5b95a221c2e77a79052e1cd0cf6f2e38b50aadd
      https://github.com/WebKit/WebKit/commit/f5b95a221c2e77a79052e1cd0cf6f2e38b50aadd
  Author: Yijia Huang <yijia_huang at apple.com>
  Date:   2024-03-25 (Mon, 25 Mar 2024)

  Changed paths:
    A JSTests/stress/propogate-PureInt-double-use.js
    M Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
    M Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.h
    M Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
    M Source/JavaScriptCore/dfg/DFGGraph.h
    M Source/JavaScriptCore/dfg/DFGPhase.cpp
    M Source/JavaScriptCore/dfg/DFGPhase.h
    M Source/JavaScriptCore/dfg/DFGPlan.cpp
    M Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp

  Log Message:
  -----------
  [JSC] We should fix the backwards propagation for DFG node use flags after the fixup phase
https://bugs.webkit.org/show_bug.cgi?id=257949
rdar://110661900

Reviewed by Yusuke Suzuki.

Given JS code:

    let x = -1 >>> v;  // <-- [1]
    let y = x + -0.2;  // <-- [2]
    let z = y | 0;     // <-- [3]

In the BackwardsPropagationPhase, Node {y} is a PureInt (The node
is never used as a number) since it's only used in [3] which will
be truncated to int32, see spec [4]. Then, the PureInt info will be
propagated to node {x} since the right operand (-0.2) at [2] is within
two to the 32th power range. Later then, the UInt32ToNumber emitted at
[1] will be optimized out in the FixupPhase due to the PureInt {x}.
However, this is not expected since {x} should be treated as a number
when being added with -0.2.

This brings us the concern that PureInt is not sufficiently proven in
the BackwardsPropagationPhase until the FixupPhase which is used to fixes
edge uses. In this case, since the {x} node has a number result because of
the emitted UInt32ToNumber, [2] wouldn't be treated as an integer add. As a
result, a DoubleRep(x) is introduced for the double arithmetic add at [2].
Here, the DoubleRep is basically a double representation which can be leveraged
by floating point arithmetic operations. With that we can confirm that the wrapped
node is definitely going to be used as a double and we should rely on that to
prove the number use of a DFG node.

So to fix this issue, we should run BackwardsPropagationPhase again after the
FixupPhase with those inserted double representation nodes as proofs in order
to fix the node uses. And move the corresponding UInt32ToNumber optimization
to the later StrengthReductionPhase.

[4] https://tc39.es/ecma262/#sec-numeric-types-number-bitwiseOR

* JSTests/stress/propogate-PureInt-double-use.js: Added.
(test):
(throw.new.Error.opt1):
(throw.new.Error.opt2):
(throw.new.Error):
* Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp:
(JSC::DFG::BackwardsPropagationPhase::BackwardsPropagationPhase):
(JSC::DFG::BackwardsPropagationPhase::propagate):
(JSC::DFG::performBackwardsPropagation):
* Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.h:
* Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* Source/JavaScriptCore/dfg/DFGGraph.h:
* Source/JavaScriptCore/dfg/DFGPhase.cpp:
(JSC::DFG::Phase::endPhase):
* Source/JavaScriptCore/dfg/DFGPhase.h:
(JSC::DFG::Phase::Phase):
* Source/JavaScriptCore/dfg/DFGPlan.cpp:
(JSC::DFG::Plan::compileInThreadImpl):
* Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:
(JSC::DFG::StrengthReductionPhase::handleNode):

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



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