On Wed, 10 Dec 2025 09:51:22 GMT, Benoît Maillard <[email protected]> wrote:
>> This PR tightens the conditions under which an optimization from >> `InlineTypeNode::Ideal` should be carried out. This optimization was >> initially reported as missed with `-XX:VerifyIterativeGVN`. >> >> ### Summary >> >> The original failure appeared with `TestFieldNullMarkers.java` and >> `-XX:VerifyIterativeGVN=1110`. This test performs various allocations with >> value classes and other Valhalla features. >> >> In `InlineTypeNode::Ideal`, we have the following optimization: >> >> https://github.com/openjdk/valhalla/blob/b1d14c658511cced2a860d9e2741ae89827e25ba/src/hotspot/share/opto/inlinetypenode.cpp#L835-L853 >> >> As explained in the optimization comments, we don't want to use the base oop >> if it corresponds to the larval oop. This is enforced by the condition >> `AllocateNode::Ideal_allocation(base) == nullptr`. >> >> In our case this is exactly what happens, and the optimization is prevented. >> HOwever, this changes during macro expansion. The allocate node disappears, >> and `AllocateNode::Ideal_allocation(base) == nullptr` becomes true. As this >> is not intended, there is no notification mechanism for this case and we end >> up with the missing optimization assert. >> >> The conditions for this bug to reproduce are somewhat subtle. When things go >> well, the larval `Allocate` node is eliminated by `eliminate_allocate_node` >> before this problematic case shows up. However, there are cases where the >> `<init>` method is not inlined, and this prevents the removal of the >> `Allocate` node. It stays until macro expansion, and that is where things go >> wrong. >> >> In the extracted reproducer, the `<init>` method is not inlined because of >> unloaded signature classes, as `CompileCommand=printinlining` shows: >> >> >> CompileCommand: PrintInlining *.* bool PrintInlining = true >> @ 5 >> compiler.valhalla.inlinetypes.TestMissingOptUseBaseOop$MyValue::<init> (10 >> bytes) failed to inline: unloaded signature classes >> >> >> This could also happen for other reasons though. >> >> ### Solution >> >> The solution is to not do this optimization after macro expansion. Adding a >> `phase->C->allow_macro_nodes()` check ensures that the >> `AllocateNode::Ideal_allocation(base)` call is relevant in the current phase >> and that we can use it to check if we are dealing with the larval oop. >> >> ### Testing >> - [x] [GitHub >> Actions](https://github.com/benoitmaillard/valhalla/actions?query=branch%3AJDK-8367623) >> - [x] tier1-4, plus some internal testing > > Benoît Maillard has updated the pull request incrementally with one > additional commit since the last revision: > > Add missing number > > Add bug reference for missed optimization in InlineType. @benoitmaillard Your change (at version 7783640579228b3eb0aa139725700da7d81c6ff6) is now ready to be sponsored by a Committer. ------------- PR Comment: https://git.openjdk.org/valhalla/pull/1742#issuecomment-3641004425
