RFR: 8371581: C2: PhaseCCP should reach fixpoint by revisiting deeply-Value-d nodes [v2]

[Quan Anh Mai]

On Thu, 13 Nov 2025 13:24:37 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> I started this as investigation into one rare/intermittent CTW failure that I get with [JDK-8360557](https://bugs.openjdk.org/browse/JDK-8360557). The bug seems to reproduce on a very specific JAR with a very specific random seed, so no easy regression test.
>> 
>> At this point I believe we found that PhaseCCP does not reach the fix point for a peculiar reason: `LoadN` that looks deeply into the graph is not revisited and thus misses the chance to update its type. There is an exception for loads in `verify_Value_for`, but it seems to only apply to constants, and does not apply to `LoadN` in question. Revisiting `LoadN` shows that updating the types downstream performs type widenings (= current types are too narrow), which AFAICS says that this unsound analysis can lead to miscompilation. See more debugging breadcrumbs in the bug.
>> 
>> It looks like we can reach the fixpoint by recording the nodes we need to revisit and doing another CCP round. This also makes CCP verification stricter: we effectively move 2 exceptional cases recorded in `verify_Value_for` into the analysis itself.
>> 
>> Testing shows there are no ill effects on correctness doing this. But I would appreciate someone more savvy in this code to sanity check all of this.
>> 
>> Additional testing:
>>  - [x] Linux x86_64 server fastdebug, CTW reproducer no longer fails
>>  - [x] Linux x86_64 server fastdebug, `all` tests pass
>>  - [x] Linux x86_64 server fastdebug, Maven Central CTW passes (!)
>
> Aleksey Shipilev has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - More restrictive CmpP check
>  - Tighten up comments and signatures
>  - Do Value() once

Thanks, LGTM.

Marked as reviewed by qamai (Committer).

src/hotspot/share/opto/phaseX.cpp line 2799:

> 2797:   // This is the meat of CCP: pull from worklist; compute new value; push changes out.
> 2798: 
> 2799:   // Do the first round.

It's worth noting that because we start with everything being `Type::TOP`, this round will visit all alive nodes in the graph.

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

PR Review: https://git.openjdk.org/jdk/pull/28288#pullrequestreview-3459814499
PR Review: https://git.openjdk.org/jdk/pull/28288#pullrequestreview-3459821035
PR Review Comment: https://git.openjdk.org/jdk/pull/28288#discussion_r2523484358