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

Roberto Castañeda Lozano rcastanedalo at openjdk.org
Fri Nov 10 15:00:04 UTC 2023


On Wed, 8 Nov 2023 15:56:21 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`.
>> - Generalize `visit_uncasted_uses` to `visit_uses` to enable the functionality required in `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.
>> - Add a regression test
>> 
>> 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
>> `tier1`, `tier2`, `tier3`, `tier4`, `tier5` (windows-x64, linux-x64, linux-aarch64, macosx-x64, macosx-aarch64)
>
> Daniel Lundén 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 16 additional commits since the last revision:
> 
>  - Update main/othervm -> driver
>  - Merge remote-tracking branch 'upstream/master' into missed-opt-8313672
>  - Remove -CICompileOSR as it's only available in debug
>  - Update incorrect test summary and comment
>  - Add UnlockDiagnosticVMOptions to allow running new test on product build
>  - Ensure single callbacks in visit_uses
>  - Change bypass to is_boundary
>  - Fix typo in visit_uses comment
>  - Increase iterations to trigger C2 without -Xcomp
>  - Add visit_uses and remove flags from test
>  - ... and 6 more: https://git.openjdk.org/jdk/compare/dfa999ec...f09d9fb1

Nice analysis and solution, Daniel! Would it be possible to preserve the const qualifiers in `push_more_uses` and `push_and`? Something like https://github.com/openjdk/jdk/commit/67b107b2d2c72fb50272408e171bc02077775005.

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

Changes requested by rcastanedalo (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16429#pullrequestreview-1724965283


More information about the hotspot-compiler-dev mailing list