RFR: 8313672: C2: PhaseCCP does not correctly track analysis dependencies involving shift, convert, and mask [v3]

[Emanuel Peter]

On Tue, 31 Oct 2023 10:19:02 GMT, Daniel Lundén <duke at openjdk.org> wrote:

>> This changeset fixes an issue where the `PhaseCCP` analysis stopped before fixpoint due to `Value`-updates for `AndL` nodes involving shift, convert, and mask (`MulNode::AndIL_shift_and_mask_is_always_zero`). Such updates did not previously repopulate the `PhaseCCP` worklist correctly.
>> 
>> Changes:
>> - Fix the issue by adding the missing pattern to `PhaseCCP::push_and`.
>> - Update the `assert` error message that reported the issue to better describe the problem. The `PhaseCCP` analysis is optimistic, and the previous error message seemed to indicate the opposite ("Missed optimization opportunity"). If the assert triggers, it could be that the analysis result leads to unsound transformations later on.
>> 
>> Result after `PhaseCCP` analysis (without fix, note `long:0` for node 116):
>> ![before](https://github.com/openjdk/jdk/assets/4222397/3f2343da-8573-464d-9352-a147d0faeab8)
>> 
>> Result after `PhaseCCP` analysis (with fix, note `long` for node 116):
>> ![after](https://github.com/openjdk/jdk/assets/4222397/6bf44964-cff8-49db-9825-4a4667f4e769)
>> 
>> ### Testing (on all Oracle-supported platforms)
>> - `tier1`
>> - HotSpot parts of `tier2` and `tier3`
>
> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix push_and to step through all ConvI2L children

test/hotspot/jtreg/compiler/ccp/TestShiftConvertAndNotification.java line 31:

> 29:  *                   -XX:+StressIGVN -XX:-CICompileOSR -Xcomp
> 30:  *                   -XX:CompileCommand=compileonly,TestShiftConvertAndNotification::test
> 31:  *                   compiler.ccp.TestShiftConvertAndNotification

How long does this test take to execute? `-XX:RepeatCompilation=1000` could make this test slow.

Do you need `-Xcomp`, or is `-Xbatch` sufficient? Or even without?

Also: I would add another `@run` description which runs the test with no flags, or at most `-Xcomp`. It has happened often that this triggers new bugs, maybe in combination with other flags set from the outside. And for that it is good to have a run with very few or better no flags/args at all. And the testing people like it if you don't just have multiple `@run` statements, so just duplicate the whole:


/*
 * @test
 * ..
 */

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16429#discussion_r1377384129