RFR: 8359603: Missed optimization in PhaseIterGVN for redundant ConvX2Y->ConvY2X->ConvX2Y sequences due to missing notification in PhaseIterGVN::add_users_of_use_to_worklist

[Christian Hagedorn]

On Thu, 17 Jul 2025 12:25:33 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` (again). This is similar to [JDK-8361700](https://bugs.openjdk.org/browse/JDK-8361700?filter=-1).
> 
> The optimization in question is the removal of redundant `ConvX2Y->ConvY2X->ConvX2Y` sequences (where `X` and `Y` are primitive number types), which get replaced by a single `ConvX2Y` as an identity optimization. This missing optimization was originally reported only for `ConvD2LNode`, but it turns out that other conversion nodes have analog optimization patterns. After manual inspection of identity optimizations in `convertnode.cpp`, I was able to reproduce missing optimizations for the following conversion sequences:
> - `ConvD2L->ConvL2D->ConvD2L`
> - `ConvF2I->ConvI2F->ConvF2I`
> - `ConvF2L->ConvL2F->ConvF2L`
> - `ConvI2F->ConvF2I->ConvI2F`
> 
> Similar optimization patterns exist for additional conversion nodes. However, it is not clear if these nodes are subject to the same missed optimization issue. Further investigation may be needed, as I was unable to reproduce such cases with simple tests.
> 
> This is again a case where an optimization depends on the input of its input. 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-8359603)
> - [x] tier1-3, plus some internal testing
> - [x] Added test from the fuzzer, and tests for other sequences (manually derived from the original one)
> 
> Thank you for reviewing!

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

> 2563:   // ConvF2I->ConvI2F->ConvF2I
> 2564:   // ConvF2L->ConvL2F->ConvF2L
> 2565:   // ConvI2F->ConvF2I->ConvI2F

Another thought: Since this is an incomplete list of variations (especially missing, for example, the I2D version while the I2F version is here), should we leave a comment about not being able to trigger issues with the other versions? Otherwise, it could suggest that it was just forgotten.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26368#discussion_r2224869135