On Wed, 22 Oct 2025 13:55:03 GMT, Christian Hagedorn <[email protected]> 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 Tobias for your review! ------------- PR Comment: https://git.openjdk.org/valhalla/pull/1694#issuecomment-3436781011
