RFR: 8361700: Missed optimization in PhaseIterGVN for mask and shift patterns due to missing notification in PhaseIterGVN::add_users_of_use_to_worklist [v2]

[Jasmine Karthikeyan]

On Wed, 16 Jul 2025 15:07:59 GMT, Benoît Maillard <bmaillard at openjdk.org> wrote:

>> This PR addresses a missed optimization in `PhaseIterGVN` due to the lack of change notification to indirect users within `PhaseIterGVN::add_users_of_use_to_worklist`. 
>> 
>> The affected optimization is the transformation of `(x & mask) >> shift` into `(x >> shift) & (mask >> shift)`, where `mask` is a constant. This transformation is handled in `RShiftNode::IdealIL` for both `RShiftI` and `RShiftL` nodes. 
>> 
>> The dependency of this optimization extends beyond a direct input: from the viewpoint of a shift node, it relies on changes to the inputs of its inputs (i.e., an `AndI`/`AndL` input node to the shift). Therefore, when the `And` node changes, the corresponding shift node must be notified to allow the optimization to take place. 
>> 
>> Currently, `PhaseIterGVN::add_users_of_use_to_worklist` contains specific logic to handle similar dependencies for other cases, but this specific scenario is not addressed. The proposed fix adds the necessary logic in `add_users_of_use_to_worklist` to ensure proper notification for this optimization pattern. 
>> 
>> ### Testing
>> - [x] [GitHub Actions](https://github.com/benoitmaillard/jdk/actions?query=branch%3AJDK-8361700)
>> - [x] tier1-3, plus some internal testing
>> - [x] Added test from the fuzzer
>> 
>> Thank you for reviewing!
>
> Benoît Maillard has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Update test/hotspot/jtreg/compiler/c2/TestMaskAndRShiftReorder.java
>    
>    Co-authored-by: Manuel Hässig <manuel at haessig.org>
>  - Update test/hotspot/jtreg/compiler/c2/TestMaskAndRShiftReorder.java
>    
>    Co-authored-by: Manuel Hässig <manuel at haessig.org>
>  - Update src/hotspot/share/opto/phaseX.cpp
>    
>    Co-authored-by: Manuel Hässig <manuel at haessig.org>

I think this is a nice fix for the notification issue. I just have one code style comment.

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

> 2554:     for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) {
> 2555:       Node* u = use->fast_out(i2);
> 2556:       if (u->Opcode() == Op_RShiftI || u->Opcode() == Op_RShiftL ) {

Suggestion:

      if (u->Opcode() == Op_RShiftI || u->Opcode() == Op_RShiftL) {

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

Marked as reviewed by jkarthikeyan (Committer).

PR Review: https://git.openjdk.org/jdk/pull/26347#pullrequestreview-3029700362
PR Review Comment: https://git.openjdk.org/jdk/pull/26347#discussion_r2213451788