RFR: 8361700: Missed optimization in PhaseIterGVN for mask and shift patterns due to missing notification in PhaseIterGVN::add_users_of_use_to_worklist
Marc Chevalier
mchevalier at openjdk.org
Wed Jul 16 14:37:41 UTC 2025
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!
Yet another of this kind. Works for me!
Would be good if we could declare what shape we are looking for, that would be used by the node idealization, and by the IGVN witchcraft to guess how deep nodes look, and update the relevant nodes automatically, without having to make it manually. I think I've read some similar vague idea before in a JBS issue, maybe from Roland?
src/hotspot/share/opto/phaseX.cpp line 2552:
> 2550: }
> 2551: }
> 2552: // If changed AndI/AndL inputs, check RShift users for "(x & mask) >> shift" reordering
"reordering" sounds not quite right to me, but I don't have a much better idea.
-------------
Marked as reviewed by mchevalier (Committer).
PR Review: https://git.openjdk.org/jdk/pull/26347#pullrequestreview-3025313234
PR Review Comment: https://git.openjdk.org/jdk/pull/26347#discussion_r2210610118
More information about the hotspot-compiler-dev
mailing list