RFR: 8359344: C2: Malformed control flow after intrinsic bailout [v2]

[Marc Chevalier]

> When intrinsic bailout, we assume that the control in the `LibraryCallKit` did not change:
> 
> https://github.com/openjdk/jdk/blob/c4fb00a7be51c7a05a29d3d57d787feb5c698ddf/src/hotspot/share/opto/library_call.cpp#L137
> 
> This is enforced by restoring the old state, like in
> 
> https://github.com/openjdk/jdk/blob/c4fb00a7be51c7a05a29d3d57d787feb5c698ddf/src/hotspot/share/opto/library_call.cpp#L1722-L1732
> 
> That is good, but not sufficient. First, the most obvious, one could have already built some structure without moving the control. For instance, we can obtain something such as:
> 
> ![1 after-intrinsic-bailout-during-late-inlining](https://github.com/user-attachments/assets/2fd255cc-0bfc-4841-8dd1-f64d502e0ee1)
> 
> 
> Here, during late inlining, the call `323` is candidate to be inline, but that bails out. Yet, a call to `make_unsafe_address` was made, which built nodes `354 If` and everything under. This is needed as tests are made on the resulting nodes (especially `366 AddP`) to know whether we should bail out or not. At the end, we get 2 control successor to `346 IfFalse`: the call that is not removed and the leftover of the intrinsic that will be cleanup much later, but not by RemoveUseless.
> 
> Another situation is somewhat worse, when happening during parsing. It can lead to such cases:
> 
> ![2 after-intrinsic-bailout-during-parsing](https://github.com/user-attachments/assets/4524c615-6521-4f0d-8f61-c426f9179035)
> 
> The nodes `31 OpaqueNotNull`, `31 If`, `36 IfTrue`, `33 IfFalse`, `35 Halt`, `44 If`, `45 IfTrue`, `46 IfFalse` are leftover from a bailing out intrinsic. The replacement call `49 CallStaticJava` should come just under `5 Parm`, but the control was updated and the call is actually built under `36 If`. Then, why does the previous assert doesn't complain?
> 
> This is because there is more than one control, or one map. In intrinsics that need to restore their state, the initial `SafePoint` map is cloned, the clone is kept aside, and if needed (bailing out), we set the current map to this saved clone. But there is another map from which the one of the `LibraryCallKit` comes, and that survives longer, it's the one that is contained in the `JVMState`:
> 
> https://github.com/openjdk/jdk/blob/c4fb00a7be51c7a05a29d3d57d787feb5c698ddf/src/hotspot/share/opto/library_call.cpp#L101-L102
> 
> And here there is the challenge:
> - the `JVMState jvms` contains a `SafePoint` map, this map must have `jvms` as `jvms` (pointer comparison)
> - we can't really change the pointer, just the content
> -...

Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:

  Remove useless loop

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/25936/files
  - new: https://git.openjdk.org/jdk/pull/25936/files/54b07e94..d51853ca

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=25936&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=25936&range=00-01

  Stats: 24 lines in 1 file changed: 0 ins; 2 del; 22 mod
  Patch: https://git.openjdk.org/jdk/pull/25936.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/25936/head:pull/25936

PR: https://git.openjdk.org/jdk/pull/25936