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

[Tobias Hartmann]

On Wed, 12 Feb 2025 16:31:02 GMT, Quan Anh Mai <qamai 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.

The fix looks reasonable to me. Just wondering, as an alternative, could we weaken the assert by checking if the call is to `OptoRuntime::rethrow_stub`?

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

PR Review: https://git.openjdk.org/valhalla/pull/1363#pullrequestreview-2623018176