RFR: 8336906: C2: assert(bb->is_reachable()) failed: getting result from unreachable basicblock [v3]

[Dean Long]

On Tue, 13 May 2025 12:39:15 GMT, Manuel Hässig <mhaessig at openjdk.org> wrote:

>> Ah, I think there is a misunderstanding:
>> I am saying that `if_cmp` does not always continue. Your statement seems to suggest that all the ones you return `true` for "continue at the next bytecode". That is missing some nuance. They `can` continue to there, but they do not always. I'm just asking for the wording to be more precise.
>> 
>> You may even want to change the name of the whole function. `falls_through` suggests that they would always fall through. But you are rather asking for "does not continue at next bci", `has_no_fallthrough` or similar.
>> 
>> I leave it up to you if / what you want to do here :)
>
> Ah, I got confused again between "should be in `falls_through()`" and "can trigger this assert". `if_cmp` cannot trigger the assert, but could be in `falls_through()`. Since it can not fail as per our current understanding and because it does not fit the current semantics of `falls_through()` I would opt to leave it as it is.

Yes, the function name and comment aren't as precise as they could be.  "parsing" might have been a better word than "execution", for example.  But I think the function name is OK as it is.  We use variants of this naming elsewhere for exactly the same purpose:
https://github.com/openjdk/jdk/blob/e7ce661adb01fba4bb690d51cc2858c822008654/src/hotspot/share/oops/generateOopMap.cpp#L1159

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25118#discussion_r2087587281