RFR: 8350563: C2 compilation fails because PhaseCCP does not reach a fixpoint [v2]

[Emanuel Peter]

On Thu, 6 Mar 2025 16:05:38 GMT, Liam Miller-Cushon <cushon at openjdk.org> wrote:

>> Hello, please consider this fix for [JDK-8350563](https://bugs.openjdk.org/browse/JDK-8350563) contributed by my colleague Matthias Ernst.
>> 
>> https://github.com/openjdk/jdk/pull/22856 introduced a new `Value()` optimization for the pattern `AndIL(Con, Mask)`.
>> This optimization can look through CastNodes, and therefore requires additional logic in CCP to push these
>> transitive uses to the worklist.
>> 
>> The optimization is closely related to analogous optimizations for SHIFT nodes, and we also extend the existing logic for
>> CCP worklist handling: the current logic is "if the shift input to a SHIFT node changes, push indirect AND node uses to the CCP worklist".
>> We extend it by adding "if the (new) type of a node is an IntegerType that `is_con, ...` to the predicate.
>
> Liam Miller-Cushon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
> 
>  - copyright
>  - style
>  - Merge branch 'openjdk:master' into mernst/JDK-8350563
>  - RegTest
>  - Merge branch 'openjdk:master' into mernst/JDK-8350563
>  - push `con->(cast*)->and` uses

Thanks for the updates! It looks much better :)

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

> 2006:     ((use_op == Op_LShiftI || use_op == Op_LShiftL) && use->in(2) == parent)) {
> 2007: 
> 2008:     auto push_and_uses_to_worklist = [&](Node* n) {

Amazing, this looks much better. I suggest you rename `new_type` -> `parent_type`, just to keep things consistent.

test/hotspot/jtreg/compiler/c2/TestAndConZeroCCP.java line 1:

> 1: /*

The test should probably be moved to `test/hotspot/jtreg/compiler/ccp/`, that would be more specific.

test/hotspot/jtreg/compiler/c2/TestAndConZeroCCP.java line 28:

> 26:  * @bug 8350563
> 27:  * @summary Test that And nodes are added to the CCP worklist if they have a constant as input.
> 28:  * @run main/othervm -Xbatch -XX:-TieredCompilation compiler.c2.TestAndConZeroCCP

Suggestion:

 * @run main/othervm -Xbatch -XX:-TieredCompilation compiler.c2.TestAndConZeroCCP
  * @run driver compiler.c2.TestAndConZeroCCP

That way we also have a run without flags, just in case that triggers some other (unrelated) bug.

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23871#pullrequestreview-2678231141
PR Review Comment: https://git.openjdk.org/jdk/pull/23871#discussion_r1991363356
PR Review Comment: https://git.openjdk.org/jdk/pull/23871#discussion_r1991360581
PR Review Comment: https://git.openjdk.org/jdk/pull/23871#discussion_r1991358419