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

Emanuel Peter epeter at openjdk.org
Tue Nov 14 09:18:37 UTC 2023


On Tue, 14 Nov 2023 09:12:44 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 incrementally with two additional commits since the last revision:
> 
>  - Move visited check to worklist push
>  - Add visited test before worklist push

src/hotspot/share/opto/node.hpp line 1654:

> 1652:     Node* use = worklist.pop();
> 1653:     // Apply callback on boundary nodes
> 1654:     if (is_boundary(use)) { callback(use); }

The if-block belongs on a new line now, since you have an else statement

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

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


More information about the hotspot-compiler-dev mailing list