RFR: 8361144: Strenghten the Ideal Verification in PhaseIterGVN::verify_Ideal_for by comparing the hash of a node before and after Ideal
Damon Fenacci
dfenacci at openjdk.org
Wed Jul 2 07:05:40 UTC 2025
On Tue, 1 Jul 2025 11:35:06 GMT, Benoît Maillard <bmaillard at openjdk.org> wrote:
> This PR adds a node hash comparison after calling `Ideal` in `PhaseIterGVN::verify_Ideal_for` to introduce an additional layer of verification for missed optimizations. Previously, we relied on the return value of `Ideal`, which is expected to be `nullptr` if no transformation was done.
>
> By also checking the node's hash before and after `Ideal`, we could catch inconsistencies in the implementation or unintended modifications to the graph. Both of these may indicate missed or incomplete optimizations.
>
> ### Testing
> - [x] [GitHub Actions](https://github.com/benoitmaillard/jdk/actions?query=branch%3AJDK-8361144)
> - [x] tier1-3, plus some internal testing
>
> Thank you for reviewing!
Thanks @benoitmaillard! Definitely an additional check worth doing. I left a couple of inline comments.
src/hotspot/share/opto/phaseX.cpp line 1821:
> 1819: // The number of nodes shoud not increase.
> 1820: uint old_unique = C->unique();
> 1821: uint old_hash = n->hash();
Just to be consistent with `old_unique` we could add a small comment (here or below for both). What do you think?
src/hotspot/share/opto/phaseX.cpp line 1838:
> 1836: stringStream ss; // Print as a block without tty lock.
> 1837: ss.cr();
> 1838: ss.print_cr("Ideal optimization did not make progress but hash node changed.");
Suggestion:
ss.print_cr("Ideal optimization did not make progress but node hash changed.");
-------------
PR Review: https://git.openjdk.org/jdk/pull/26064#pullrequestreview-2977964471
PR Review Comment: https://git.openjdk.org/jdk/pull/26064#discussion_r2179270798
PR Review Comment: https://git.openjdk.org/jdk/pull/26064#discussion_r2179279429
More information about the hotspot-compiler-dev
mailing list