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

[Daniel Lundén]

On Mon, 7 Apr 2025 11:44:49 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Clean up code related to eager transformation and rename itergvn to igvn
>
> src/hotspot/share/opto/phaseX.cpp line 1058:
> 
>> 1056:       // Ensure we did not increase the live node count with more than
>> 1057:       // max_live_nodes_increase_per_iteration during the call to transform_old
>> 1058:       DEBUG_ONLY(int increase = live_nodes_after - live_nodes_before;)
> 
> For consistency with the surrounding code, maybe you could define these as `NOT_PRODUCT`, and possibly group them under `#ifndef PRODUCT` blocks?

As we discussed offline, there is a subtle difference between `NOT_PRODUCT` and `DEBUG_ONLY`. `NOT_PRODUCT` also runs in non-product "optimized" builds where asserts are disabled, and I really only want this code to run when asserts are enabled. Therefore, I believe `DEBUG_ONLY` (or alternatively `#ifdef ASSERT`) is most suitable here.

> test/hotspot/jtreg/compiler/itergvn/TestSplitPhiThroughMergeMem.java line 67:
> 
>> 65:             new String("abcdef" + param2);
>> 66:             new String("ghijklmn" + param1);
>> 67:             new String("ghijklmn" + param1);
> 
> This test illustrates an interesting behavior: C2 generates around 12 Kb of code for this rather infrequent code path (and the frequency can be further reduced without affecting C2's outcome). This seems to contradict C2's general philosophy of focusing the compilation effort (and code cache usage) on hot code. It would be interesting to investigate whether there is an opportunity to make some heuristic more execution-frequency aware here.

Yes, for sure interesting. Let us create a separate RFE to investigate.

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

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