RFR: 8253740: [PPC64] Minor interpreter cleanup

[David Holmes]

On 29/09/2020 10:52 pm, Martin Doerr wrote:
> On Tue, 29 Sep 2020 09:12:37 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:
> 
>>> 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. (monitor_enter_helper uses the CHECK macro which does the check for an exception,
>> but this looks like a bug.)  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. At least, it'd be nice to bring the
>> interpreter and the compiler version closer together. But that's a bit out of scope for this issue.
> 
> Hi David,
> thanks for reminding me of JVMTI event callbacks. So we need to check s390 code.
> And in SharedRuntime::monitor_enter_helper it doesn't make sense to use CHECK macro and then
> assert(!HAS_PENDING_EXCEPTION, "Should have no exception here"). But this is unrelated to this PPC64 cleanup change.

The assert is somewhat redundant but not wrong, as it it is validating 
that the CHECK macro actually worked:

   ObjectSynchronizer::enter(h_obj, lock, CHECK);
   assert(!HAS_PENDING_EXCEPTION, "Should have no exception here");

expands to:

   ObjectSynchronizer::enter(h_obj, lock);
   if (HAS_PENDING_EXCEPTION)
     return;
   assert(!HAS_PENDING_EXCEPTION, "Should have no exception here");

Cheers,
David

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