RFR: 8361699: C2: assert(can_reduce_phi(n->as_Phi())) failed: Sanity: previous reducible Phi is no longer reducible before SUT

[Roberto Castañeda Lozano]

On Tue, 9 Sep 2025 07:01:15 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>>> Hi Cesar, thanks for addressing this issue. I will run some more comprehensive testing and have a look at it in the next days.
>> 
>> Testing did not reveal any issue. I have, however, a high-level question: could the current two-step design ([SR state adjustment loop](https://github.com/openjdk/jdk/blob/166ef5e7b1c6d6a9f0f1f29fedb7f65b94f53119/src/hotspot/share/opto/escape.cpp#L300-L315) followed by a [NSR propagation loop](https://github.com/openjdk/jdk/blob/166ef5e7b1c6d6a9f0f1f29fedb7f65b94f53119/src/hotspot/share/opto/escape.cpp#L318-L320) miss marking allocations as NSR in more complex scenarios, e.g. involving longer points-to/merge chains? Wouldn't it be more principled to re-run the SR state adjustment loop until a fixed point is reached, keeping `reducible_merges` consistent as new allocations are discovered to be NSR? (e.g. by calling `revisit_reducible_phi_status` - with your clean-up applied - every time [an allocation is marked as NSR due to non-removable merges](https://github.com/openjdk/jdk/blob/166ef5e7b1c6d6a9f0f1f29fedb7f65b94f53119/src/hotspot/share/opto/escape.cpp#L2962-L2964)).
>
>> @robcasloz - are you thinking that the "fixed point" loops on `find_scalar_replaceable_allocs` aren't sufficient?
> 
> You're right, that should do.
> 
>> At first glance yes, I think that the code would be more cleaned up if done that way. If the code had been written like that in the first place we wouldn't have seen the current issue. (...)
> 
> Agree, a single fixed point loop combining NSR detection and propagation would be ideal for clarity and maintainability.
> 
>>  I propose that we move forward with the current patch and work on this refactoring as a separate issue.
> 
> Sounds good, please file a RFE for that. I would suggest then to postpone the clean-up in `revisit_reducible_phi_status` to that RFE.

> @robcasloz - I pushed some changes addressing yours and @eme64 comments. Could you please re-run your internal tests?

Thanks, I will report back within a couple of days.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/27063#issuecomment-3278934984