[jdk18] RFR: 8275638: GraphKit::combine_exception_states fails with "matching stack sizes" assert

[Vladimir Ivanov]

On Mon, 3 Jan 2022 08:29:08 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> Wouldn't that only work if there's no uncommon trap in GraphKit::builtin_throw()?

Good point. I overlooked the uncommon trap logic there. 

But my original point still stands: why can't `GraphKit::builtin_throw()` be taught to distinguish between throwing and uncommon trap cases and adjust JVM state accordingly? In this particular case, it seems simpler to start with the JVM state for re-execution (before call) and switch to the JVM state after the call (pop arguments from the stack) when an exception is thrown.

So, instead of checking for local exception handlers and wiping the stack [1], the effects of `null_check_receiver_before_call()` [2] can be undone unconditionally. That should make all possible exception states uniform and, hence, mergeable, shouldn't it?    

[1]

       if (!method()->has_exception_handlers()) {
        // We don't need to preserve the stack if there's no handler as the entire frame is going to be popped anyway.
        // This prevents issues with exception handling and late inlining.
        set_sp(0);
        clean_stack(0);
      }
``` 

[2] 

  // Do a null check on the receiver as it would happen before the call to
  // callee (with all arguments still on the stack).
  Node* null_check_receiver_before_call(ciMethod* callee) {
    assert(!callee->is_static(), "must be a virtual method");
    // Callsite signature can be different from actual method being called (i.e _linkTo* sites).
    // Use callsite signature always.
    ciMethod* declared_method = method()->get_method_at_bci(bci());
    const int nargs = declared_method->arg_size();
    inc_sp(nargs);
    Node* n = null_check_receiver();
    dec_sp(nargs);
    return n;
  }

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

PR: https://git.openjdk.java.net/jdk18/pull/29