RFR: 8253740: [PPC64] Minor interpreter cleanup

[Martin Doerr]

On Tue, 29 Sep 2020 12:50:12 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> 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.

Hi Coleen,
I've removed the explicit check_exceptions=true parameters on PPC64 so it looks more like x86.
I'll take a look at s390 separately.

Hi David,
the assert(!HAS_PENDING_EXCEPTION, "Should have no exception here") is not wrong, but confusing IMHO.

Thanks for looking at this.

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

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