Integrated: 8371065: C2 SuperWord: VTransformLoopPhiNode::apply setting type leads to assert/wrong result
Emanuel Peter
epeter at openjdk.org
Wed Nov 12 07:13:23 UTC 2025
On Mon, 3 Nov 2025 15:20:37 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
> In `VTransformLoopPhiNode::apply`, we may have to modify the type of the phi node, because it may have been turned from a scalar phi to a vector phi by `VTransformReductionVectorNode::optimize_move_non_strict_order_reductions_out_of_loop`. This logic was refactored in https://github.com/openjdk/jdk/pull/27704, and I missed some edge cases that the fuzzer now found.
>
> - The first issue: when we (uslessly) set the type of phis that stay scalar: the `in1` type can be a constant, and then we set the `phi` type to be constant. And then the phi wrongly constant folds. That leads to wrong results.
> - The second issue: a phi that was scalar and we turned into vector still had some dead old scalar reduction nodes attached. They would of course eventually die during IGVN. But with `StressIGVN` just picking the right bad order, it could happen that an `AddI` attached to the `phi` would try to figure out its `Value` type, and try to combine the vector type of the `phi` with the other input, leading to a type error.
>
> With only the first issue at first, I tried to improve the way we modify the type from scalar to vector. But with the second issue, it became clear that we should just create a new phi node when we move from scalar to vector phi. Hence, I split the `LoopPhi` into a `PhiScalar` and a `PhiVector`, and give them separate implementations.
>
> ---------
>
> Thanks @rwestrel for filing this issue and spending a lot of time reproducing it without his changes.
> I tried to find a simpler reproducer, but it was difficult: We need a constant on the lhs of the phi in the main-loop. But this requires us to constant-fold the pre-loop phi, and somehow magically not constant fold the phi of the main-loop. That is quite tricky, and I gave up.
>
> Later, the fuzzer found the second reproducer on mainline, which was much easier to reduce.
This pull request has now been integrated.
Changeset: 6df78c45
Author: Emanuel Peter <epeter at openjdk.org>
URL: https://git.openjdk.org/jdk/commit/6df78c4585fc5a71ceafa6f4b1dc0fe68db2657c
Stats: 290 lines in 4 files changed: 257 ins; 7 del; 26 mod
8371065: C2 SuperWord: VTransformLoopPhiNode::apply setting type leads to assert/wrong result
Co-authored-by: Roland Westrelin <roland at openjdk.org>
Reviewed-by: qamai, chagedorn
-------------
PR: https://git.openjdk.org/jdk/pull/28113
More information about the hotspot-compiler-dev
mailing list