RFR: 8253740: [PPC64] Minor interpreter cleanup

[David Holmes]

Hi Martin,

On 29/09/2020 7:14 pm, Martin Doerr wrote:
> On Tue, 29 Sep 2020 00:02:44 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
> 
>>> Template interpreter on PPC64 can be cleaned up after JDK-8253540:
>>> unlock_object has a parameter check_for_exceptions which is now unused.
>>>
>>> In addition to that, restore_interpreter_state is redundant after
>>> call_VM(R4_ARG2, CAST_FROM_FN_PTR(address, InterpreterRuntime::member_name_arg_or_null) because it's already included
>>> in call_VM.
>>> https://bugs.openjdk.java.net/browse/JDK-8253740
>>
>> This looks good but I also noticed that ppc and s390 pass an extra parameter to InterpreterRuntime::monitorenter() also.
> 
> That's an interesting point: call_VM(monitorenter) uses check_for_exceptions=true on PPC64 and
> check_for_exceptions=false on s390. check_for_exceptions=true is default, so it's equivalent to omitting the explicit
> parameter. So PPC64 is implemented like x86 in this matter. Seems like we have optimized the exception check out for
> s390. This should be valid because monitorenter uses JRT_ENTRY_NO_ASYNC and doesn't throw any exception. Note that the
> JIT compiler's version uses assert(!HAS_PENDING_EXCEPTION, "Should have no exception here") in
> SharedRuntime::monitor_enter_helper. I think we could assert that in the interpreter's version and use
> check_for_exceptions=false on all platforms. It's probably not worth optimizing this, but maybe it'd be a good
> simplification wrt. complexity and readability of the gererated assembly code. But that's a bit out of scope for this
> issue.

It is not true that monitorenter can't throw exceptions. Contended 
monitorenter posts a JVMTI event and the event callback can execute Java 
code that triggers an exception. The "no async" just means the callVM 
won't actively looking for a pending-async to install, but there could 
already be an exception in flight (async or otherwise).

Cheers,
David


> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/385
>