RFR: 8361140: Missing OptimizePtrCompare check in ConnectionGraph::reduce_phi_on_cmp

[guanqiang han]

On Mon, 7 Jul 2025 12:11:46 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> When running with `-XX:-OptimizePtrCompare` (which disables pointer comparison optimization), the compiler may hit an assertion failure in debug builds because `optimize_ptr_compare` is still being called. This violates the intended usage of the flag and leads to unexpected crashes.
>> 
>> This patch adds an early return to `reduce_phi_on_cmp` when `OptimizePtrCompare` is false. Since the optimization relies on `optimize_ptr_compare` for static reasoning about comparisons, there's no benefit in proceeding with `reduce_phi_on_cmp` when this flag is disabled.
>
> src/hotspot/share/opto/escape.cpp line 981:
> 
>> 979:   if (!OptimizePtrCompare) {
>> 980:     return;
>> 981:   }
> 
> Thanks for working on this! IIUC, having the bailout here will fail to reduce the phi which could be unexpected. Shouldn't we just return `UNKNOWN` from within `ConnectionGraph::optimize_ptr_compare()` when we run without `OptimizePtrCompare`?
> 
> On a separate note, can you also add a regression test? Maybe you can also just add a run with `-XX:-OptimizePtrCompare` - maybe together with `-XX:+VerifyReduceAllocationMerges` for more verification - to `compiler/c2/irTests/scalarReplacement/AllocationMergesTests.java`.
> 
> @JohnTortugo you might also want to have a look at this.

Thanks a lot for your suggestion!
I took a closer look at the code, and I now fully agree that your approach is the better one. Returning UNKNOWN from optimize_ptr_compare() when OptimizePtrCompare is disabled makes the behavior more consistent and avoids skipping reduce_phi_on_cmp() entirely, which could lead to unexpected results or missed optimization opportunities. I appreciate your feedback and will move forward with this approach. Thanks again!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26125#discussion_r2190428742