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

[Manuel Hässig]

On Wed, 16 Jul 2025 12:42:32 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!

Thank you for working on this, @benoitmaillard. I only have a few nits. Otherwise, this looks good to me.

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

> 26:  * @bug 8361700
> 27:  * @summary An expression of the form "(x & mask) >> shift", where the mask
> 28:  *          is a constant, should be reordered to "(x >> shift) & (mask >> shift)"

Suggestion:

 *          is a constant, should be transformed to "(x >> shift) & (mask >> shift)"

I agree with @marc-chevalier that "reordered" is not the right word.

test/hotspot/jtreg/compiler/c2/TestMaskAndRShiftReorder.java line 60:

> 58:         return iArr.length;
> 59:     }
> 60: }

Suggestion:

}

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

Changes requested by mhaessig (Committer).

PR Review: https://git.openjdk.org/jdk/pull/26347#pullrequestreview-3025384023
PR Review Comment: https://git.openjdk.org/jdk/pull/26347#discussion_r2210669477
PR Review Comment: https://git.openjdk.org/jdk/pull/26347#discussion_r2210671082