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

[Cesar Soares Lucas]

On Mon, 8 Sep 2025 15:38:52 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.
>
>> 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? 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. But I don't think this is a correctness issue. As long as we call `revisit_reducible_phi_status` when an object is marked as NSR the eventual call to `unique_java_object` should find that NSR object if it's used by a reducible phi. I propose that we move forward with the current patch and work on this refactoring as a separate issue.

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

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