RFR: 8371065: C2 SuperWord: VTransformLoopPhiNode::apply set wrong type, led to wrong constant folding of phi [v2]
Quan Anh Mai
qamai at openjdk.org
Sat Nov 8 11:44:01 UTC 2025
On Fri, 7 Nov 2025 06:07:22 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 an edges case.
>>
>> The issue is when we also (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.
>>
>> I now limit the modification to cases where the `phi` used to be for scalars, but now is for vectors. In those cases we should not have a constant. For good measure, I also added a corresponding assert.
>>
>> ---------
>>
>> 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.
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>
> add diagnostic flag for product build
Tbh I still feel a little uneasy with this, what if in the future we try to vectorize to a `long` sometimes, too? Is there anything stopping us from creating a new `Phi` for the `VTransformLoopPhiNode` instead?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/28113#issuecomment-3506472710
More information about the hotspot-compiler-dev
mailing list