RFR: 8291555: Implement alternative fast-locking scheme [v62]

[Roman Kennke]

On Wed, 26 Apr 2023 03:09:53 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove unnecessary comments
>
> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 691:
> 
>> 689:   jccb(Assembler::notEqual, NO_COUNT);    // If not recursive, ZF = 0 at this point (fail)
>> 690:   incq(Address(scrReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(recursions)));
>> 691:   xorq(rax, rax); // Set ZF = 1 (success) for recursive lock, denoting locking success
> 
> `xorl` would save a byte here, and some similar places.

Yes, but see above.

> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 701:
> 
>> 699:   // ZFlag == 0 count in slow path
>> 700:   jccb(Assembler::notZero, NO_COUNT); // jump if ZFlag == 0
>> 701: 
> 
> `DONE_LABEL` is conditionally jumped into from a lot of places, the only path it is reached without known `ZF` seems to be `LM_LEGAGY` fall-through. Maybe refactor a little to eliminate this block.

I intentionally have not changed the existing paths to make it absolutely clear that the old behaviour is not changed. I'd  rather make any changes to the stack-locking in a separate follow-up.

> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 926:
> 
>> 924:     // Intentional fall-thru into DONE_LABEL
>> 925:   }
>> 926:   bind(DONE_LABEL);
> 
> Similar to `fast_lock`, this `DONE_LABEL` can be removed.

Yes but see above ;-)

> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 9704:
> 
>> 9702: 
>> 9703:   // If successful, push object to lock-stack.
>> 9704:   movl(tmp, Address(thread, JavaThread::lock_stack_top_offset()));
> 
> This value seems to be loaded twice, can they be merged?

That would be nice, but we cannot do this without allocating another tmp register.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1177695188
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1177695040
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1177700194
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1177696506