RFR: 8329797: Shenandoah: Default case invoked for: "MaxL" (bad AD file) [v4]

[Tobias Hartmann]

On Fri, 19 Apr 2024 17:16:24 GMT, Joshua Cao <duke at openjdk.org> wrote:

>> The bug occurs when [Shenandoah optimizations resets post_loop_opts](https://github.com/openjdk/jdk/blob/040c93565c0dff6270911eb9e58d78aa01bbb925/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp#L52), and we may [create a MaxL after macro expansion](https://github.com/openjdk/jdk/blob/040c93565c0dff6270911eb9e58d78aa01bbb925/src/hotspot/share/opto/movenode.cpp#L198). `MaxL` does not have a matcher rule, and we run into an assertion failure.
>> 
>> This PR guards the `MaxL` creation with a new `began_macro_expansion()` flag. I think there are many other instances in code that should use the new flag instead of `post_loop_opts()`, which can be explored in [JDK-8330531](https://bugs.openjdk.org/browse/JDK-8330531).
>> 
>> The bug was originally found in [h2 Index::getCostRangeIndex()](https://github.com/h2database/h2database/blob/master/h2/src/main/org/h2/index/Index.java#L579) through Dacapo. Its easy to reproduce by creating a loop that includes a `ShenandoahLoadReferenceBarrier` (load any object) and a `MaxL`.
>> 
>> Caveat: I created test cases for both `MaxL` and `MinL` for completeness. The `MinL` test case does not actually fail before this PR. Somehow the `CMove` condition is converted to non-canonical `>`, which is [not accepted by the Idealization](https://github.com/openjdk/jdk/blob/040c93565c0dff6270911eb9e58d78aa01bbb925/src/hotspot/share/opto/movenode.cpp#L219). The `MinL` is never created and there is no crash.
>> 
>> Passing hotspot tier1 locally on Linux machine.
>
> Joshua Cao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Comment on not allowing macro nodes after we start expanding. Rename
>   dont_allow_macro_nodes to reset_allow_macro_nodes.

Looks good to me. I submitted testing and will report back once it passed.

Please adjust the description of [JDK-8330531](https://bugs.openjdk.org/browse/JDK-8330531) according to the new naming.

> Somehow the CMove condition is converted to non-canonical >, which is [not accepted by the Idealization](https://github.com/openjdk/jdk/blob/040c93565c0dff6270911eb9e58d78aa01bbb925/src/hotspot/share/opto/movenode.cpp#L219). The MinL is never created and there is no crash.

Is the problem that the condition is not canonicalized or that the CMoveNode is not process by IGVN after canonicalization of the cmp?

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18824#pullrequestreview-2014424503