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

Joshua Cao duke at openjdk.org
Fri Apr 19 16:32:56 UTC 2024


On Fri, 19 Apr 2024 08:37:05 GMT, Aleksey Shipilev <shade 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.
>
> src/hotspot/share/opto/compile.hpp line 321:
> 
>> 319: 
>> 320:   bool                  _post_loop_opts_phase;  // Loop opts are finished.
>> 321:   bool                  _began_macro_expansion; // Macro expansion is started.
> 
> I think this sounds better as inverse, `bool _allow_macro_nodes; // Allow creating macro nodes`.  Then we can also assert `_allow_macro_nodes` in `add_macro_node`?

Can't add the assert for free. Apparently macro nodes are added later in the compiler pipeline, Matcher in this case. 


Stack: [0x00007fb2c6d50000,0x00007fb2c6e51000],  sp=0x00007fb2c6e4caa0,  free space=1010k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.so+0x63bd9d]  Compile::add_macro_node(Node*)+0x49  (compile.hpp:742)
V  [libjvm.so+0x124ccc5]  Node::clone() const+0x15f  (node.cpp:501)
V  [libjvm.so+0x119a93a]  Matcher::xform(Node*, int)+0x3a2  (matcher.cpp:1150)
V  [libjvm.so+0x1195413]  Matcher::match()+0xe8b  (matcher.cpp:359)
V  [libjvm.so+0x9a9723]  Compile::Code_Gen()+0x95  (compile.cpp:2947)
V  [libjvm.so+0x99fd8d]  Compile::Compile(ciEnv*, ciMethod*, int, Options, DirectiveSet*)+0x1827  (compile.cpp:895)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18824#discussion_r1572635814


More information about the hotspot-compiler-dev mailing list