RFR: 8319801: Recursive lightweight locking: aarch64 implementation [v6]

[Axel Boldt-Christmas]

On Wed, 24 Jan 2024 12:05:04 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> Axel Boldt-Christmas has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319801
>>  - Switch to CAS over LXSX
>>  - Fix missing $
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319801
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319801
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319801
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319801
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319801
>>  - 8319801: Recursive lightweight locking: aarch64 implementation
>>  - Cleanup: C2 fast_lock/fast_unlock aarch64
>
> src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp line 81:
> 
>> 79:     tstw(hdr, JVM_ACC_IS_VALUE_BASED_CLASS);
>> 80:     br(Assembler::NE, slow_case);
>> 81:   } else if (LockingMode == LM_LIGHTWEIGHT) {
> 
> What is the advantage of moving the load of the header around? The way you did it, it is less obvious that for LW the header is loaded in the block before locking is actually done.

The current implementation requires the first emitted instruction of `C1_MacroAssembler::lock_object` to trap if the obj is null.  We do it here with a load. 

I could instrument `MacroAssembler::lightweight_lock` with a `bool preload_mark` so that C1 does not do the redundant load, while still emitting its first trapping instruction, and not create redundant loads for the interpreter and native wrappers.

I think that we want to move all platforms to what PPC is doing, mainly that C2, C1 and native wrapper shares a common lock/unlock implementation (as they lock + unlock under the same balanced assumptions), while have `MacroAssembler::lightweight_lock` solely dedicated to the interpreter. 

I think I will update `MacroAssembler::lightweight_lock` to have this `preload_mark` bool. You are not the first to ask about this specific line, and it might make it more clear. With a comment about the C1 implicit null check.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16608#discussion_r1464999437