RFR: 8351833: Unexpected increase in live nodes when splitting Phis through MergeMems in PhiNode::Ideal [v2]

[Daniel Lundén]

On Mon, 7 Apr 2025 12:38:59 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> src/hotspot/share/opto/cfgnode.cpp line 2580:
>> 
>>> 2578:         // IGVN iteration. We have put the Phi nodes on the IGVN worklist, so
>>> 2579:         // they are transformed later on in any case.
>>> 2580:         hook->destruct(igvn);
>> 
>> Do you need this `hook` node to preserve `new_base` now that you are not calling `PhaseGVN::transform` anymore?
>
> We then probably also do not need the code directly above anymore added by [JDK-8275326](https://bugs.openjdk.org/browse/JDK-8275326) which was only required due to this eager phi transformation. Could you find out why this eager transformation was added in the first place?

Thanks for the comments! I have now cleaned up according to your suggestions and checked that it still works (tests and code inspection). I also investigated why the eager transformation was added in the first place, but was unable to find much information. It is from initial load, and looking back even further in the pre-initial-load history, I see that the code was introduced back in 2000 already. The commit message is not really helpful, unfortunately. My best guess is that eagerly transforming the Phis may perhaps result in a preferable ordering of idealizations that introduces less (temporary) nodes. The extremas for "target" and "target-old" gives an indication of this (see the plots in the PR description), but in practice it has no significant observable effect.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24325#discussion_r2037141048