[lworld] RFR: 8369805: [lworld] C2: assert(is_InlineType()) failed: invalid node class: Con

[Tobias Hartmann]

On Wed, 22 Oct 2025 13:55:03 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> `TestLWorld.java` was failing intermittently with `-XX:+StressIGVN` and `-XX:+StressLoopPeeling` due to not handling `top` correctly in `InlineTypeNode::merge_with()` during IGVN.
> 
> We have the following graph during IGVN:
> <img width="558" height="304" alt="image" src="https://github.com/user-attachments/assets/6c146038-208f-4f5a-bcfa-5be0d5b67537" />
> 
> The following steps happen:
> 1. `6837 Phi` was created in Loop Peeling to merge data nodes from the peeled iteration and the remaining loop (this is the reason we are only seeing this failure with `StressLoopPeeling` - there are simply more chances with more phis to get unlucky and hit the assert).
> 2. When processing `6837 Phi` in IGVN, we apply `PhiNode::try_push_inline_types_down()` in order to push the `InlineType` nodes down recursively. 
> 3. Doing it recursively, we will then call `PhiNode::try_push_inline_types_down()` on `6535 Phi`.
> 4. We create a new initial `InlineType` node based on the same klass as `6259 InlineType` (i.e. the same fields) but use `Phis` for the inputs or `InlineType` nodes if the fields are value objects themselves:
> https://github.com/openjdk/valhalla/blob/abc1d5b2632ea270735f1e5aa794c7ab9e5f2378/src/hotspot/share/opto/inlinetypenode.cpp#L87-L89
> This new `InlineType` node represents the resulting "pushed down" `InlineType`. Let's call it `inline-type-result`.
> 5.  We now populate the phi inputs of `inline-type-result` by visiting the `InlineType` inputs of `6535 Phi`.
> 6. We visit the first input `6259 InlineType`: We update the first input of the phis of `inline-type-result` by calling `merge_with()` on `inline-type-result`.
> 7. We visit all inputs of `inline-type-result`: These are either phis or `InlineType` nodes (see step 4). When we see an `InlineType` input, we must also see an `InlineType` input for `6259 InlineType` because we used the same klass in step 4. However, we miss here that we could also have a top input because the node is actually being removed in IGVN because we only take the other path of `6535 Phi`. Therefore this assumption is wrong:
> https://github.com/openjdk/valhalla/blob/abc1d5b2632ea270735f1e5aa794c7ab9e5f2378/src/hotspot/share/opto/inlinetypenode.cpp#L157-L161
> and we crash because `val2` is top.
> 
> The fix is straight forward to stop the merge since the node is dead. Since this is highly intermittent and occurring with `TestLWorld.java` only, I have not added a separate test case. Additionally to standard testing, I also ran `TestLWorld.java` 100 times in th...

Thanks for the thorough analysis Christian! Looks good to me.

src/hotspot/share/opto/inlinetypenode.cpp line 162:

> 160:       }
> 161:       if (val2->is_top()) {
> 162:         // The path where 'other' is used is dying. Therefore, we do not need process the merge with 'other' further.

Suggestion:

        // The path where 'other' is used is dying. Therefore, we do not need to process the merge with 'other' further.

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

Marked as reviewed by thartmann (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/1694#pullrequestreview-3369503073
PR Review Comment: https://git.openjdk.org/valhalla/pull/1694#discussion_r2454826830