RFR: 8338995: New Object to ObjectMonitor mapping: PPC64 implementation [v3]

[Richard Reingruber]

On Wed, 18 Sep 2024 10:34:31 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 2825:
>> 
>>> 2823:       for (int i = 0; i < num_unrolled; i++) {
>>> 2824:         ld(tmp3, 0, cache_addr);
>>> 2825:         cmpd(CCR0, tmp3, obj);
>> 
>> Please file a RFE to consistently use either `flag` or `CCR0`.
>
> I'm planning to remove `flag` and use `CCR0` consistently. But I'd like to postpone this and do it after the removal of the old locking modes. (Hoping this will happen soon.)

Reviews like this would be easier without the inconsistency. The sooner this is fixed the better. But that's just my humble opinion ;)

>> src/hotspot/cpu/ppc/macroAssembler_ppc.cpp line 2878:
>> 
>>> 2876:       ld(tmp2, in_bytes(ObjectMonitor::recursions_offset()), monitor);
>>> 2877:       addi(tmp2, tmp2, 1);
>>> 2878:       std(tmp2, in_bytes(ObjectMonitor::recursions_offset()), monitor);
>> 
>> Can you replace the if-statement with the following?
>> 
>> Suggestion:
>> 
>>       assert_different_registers(tmp2, monitor);
>>       int offset = in_bytes(ObjectMonitor::recursions_offset()) - (UseObjectMonitorTable ? 0 : monitor_tag);
>>       ld(tmp2, offset, monitor);
>>       addi(tmp2, tmp2, 1);
>>       std(tmp2, offset, monitor);
>
> Is `monitor` initialized as needed if `!UseObjectMonitorTable`? I can't see it.

Yes, I think so. Note that `mark` and `monitor` are the same register. The difference is that the ObjectMonitor pointer in the mark word is tagged. This needs to be compensate for in the offset (L2876 in my suggestion). It might be better to use `monitor` also at L2814 and add a comment at the declaration of `monitor` that the pointer is tagged with `!UseObjectMonitorTable`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20922#discussion_r1765045501
PR Review Comment: https://git.openjdk.org/jdk/pull/20922#discussion_r1765041298