[lworld] RFR: 8349068: [lworld] C2 compilation fails with "not enough operands for reexecution"

[Tobias Hartmann]

On Mon, 24 Feb 2025 09:13:07 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Hi,
>> 
>> The issue here is that `GraphKit::make_runtime_call` is often used to execute a bytecode. As a result, it expects that it may deoptimize and thus need to reexecute the current bytecode in the interpreter. This is the intention of the `assert`, to verify that we are having enough operands for reexecution of the bytecode. However, in the failing case, `GraphKit::make_runtime_call` is not used to execute the bytecode, but to handle the exceptions thrown by that bytecode. In this case, the bytecode itself has finished executing and must not be reexecuted, we can see that in `Parse::catch_inline_exceptions`:
>> 
>>     // Oops, need to call into the VM to resolve the klasses at runtime.
>>     // Note:  This call must not deoptimize, since it is not a real at this bci!
>>     kill_dead_locals();
>> 
>>     make_runtime_call(RC_NO_LEAF | RC_MUST_THROW,
>>                       OptoRuntime::rethrow_Type(),
>>                       OptoRuntime::rethrow_stub(),
>>                       nullptr, nullptr,
>>                       ex_node);
>> 
>> As a result, reexecution is impossible and we don't need to worry about the operand stack, I propose removing the `assert` as it seems to be the cleanest fix.
>> 
>> The reason this only fails on `lworld` is because here the execution of `aastore` involves a static Java call, resulting in a potential exception that needs catching.
>> 
>> Please share your thoughts, thanks a lot.
>
> src/hotspot/share/opto/graphKit.cpp line 941:
> 
>> 939:     assert(method() == youngest_jvms->method(), "sanity");
>> 940:     assert(compute_stack_effects(inputs, not_used), "unknown bytecode: %s", Bytecodes::name(java_bc()));
>> 941:     assert(out_jvms->sp() >= (uint)inputs, "not enough operands for reexecution");
> 
> Removal of this assertion looks incorrect, given that stack holds the active expressions this sanity check ensures that required number of inputs needed to re-construction of interpreter frame are present.

This will be fixed by [JDK-8350208](https://bugs.openjdk.org/browse/JDK-8350208) in mainline.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1363#discussion_r1969518653