On Fri, 3 Jul 2026 06:56:15 GMT, Tobias Hartmann <[email protected]> wrote:
>> In the test case, we are trying to merge a dead path in a live target block >> which is unexpected and we fail with an assertion failure. >> >> We call `Parse::do_acmp()` with `BoolTest::ne`. This means that the target >> block is reached over `ne_region`, which collects all the branching paths, >> and the fall-through block is `eq_region`, which collects all the >> fall-through paths. >> >> We emit several checks to see if we need to branch off and capture those in >> `ne_region`. In the test case, we either fold such a branch directly with >> the available information or we emit an uncommon trap in which case we also >> will not branch off to the target block. At the end of `do_acmp`, we only >> collected dead paths in the `ne_region` and thus are left with only `top` >> inputs. This means that we will not branch off to the target block and will >> fall through. We transform the `ne_region` with `top` inputs only, get `top` >> back and set it as current control: >> https://github.com/openjdk/valhalla/blob/6adeca933a1361c6f44f51c9c266bd1a20a42380/src/hotspot/share/opto/parse2.cpp#L2436 >> Nothing unusual, this is perfectly fine to signal that this path is dead. So >> far, so good. >> >> Since `do_acmp` handles both the `BoolTest::ne` and `BoolTest::eq` case, we >> actually need to swap the fall-through control to be the `eq_region` instead: >> https://github.com/openjdk/valhalla/blob/6adeca933a1361c6f44f51c9c266bd1a20a42380/src/hotspot/share/opto/parse2.cpp#L2447-L2450 >> >> Before doing that, we merge the `ne_region` into the target block: >> https://github.com/openjdk/valhalla/blob/6adeca933a1361c6f44f51c9c266bd1a20a42380/src/hotspot/share/opto/parse2.cpp#L2441-L2445 >> >> However, if the target block is live by already having merged another block >> as in the test case, we will merge a dead path with a live path. This is not >> anticipated by `merge()` and we fail with an assertion failure: The caller >> needs to make sure to only call `merge()` with live paths. >> >> The solution is straight forward: Only `merge()` if we have not `stopped()`. >> Otherwise, mark the block as handled and do nothing. >> >> Thanks, >> Christian >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > src/hotspot/share/opto/parse2.cpp line 2448: > >> 2446: PreserveJVMState pjvms(this); >> 2447: merge(target_bci); >> 2448: } else if (C->eliminate_boxing()) { > > I'm not sure how much sense that guard still makes but it's consistent with > other code so out of scope of this change. I'm also not sure, either. I added it to be consistent with other code, for example, in `do_if()`. We could further investigate in mainline if that guard is really needed or if we should do it unconditionally. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2618#discussion_r3518054926
