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

[Axel Boldt-Christmas]

On Tue, 13 Feb 2024 12:33:17 GMT, Andrew Haley <aph 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 pull request now contains 79 commits:
>> 
>>  - Merge remote-tracking branch 'upstream_jdk/master' into JDK-8319801
>>  - Revert "Update src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp"
>>    
>>    This reverts commit 130cf4b7f5b37ecb9dda3ce07ed70c3da13b29b8.
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319801
>>  - Running out of LogTagSets, stick with monitorinflation
>>  - Merge remote-tracking branch 'upstream_jdk/master' into JDK-8319797
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319801
>>  - Merge remote-tracking branch 'upstream_jdk/master' into JDK-8319797
>>  - Fix assert comment
>>  - Update src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp
>>    
>>    Co-authored-by: Andrew Haley <aph-open at littlepinkcloud.com>
>>  - Clarify the rscratch1 assert
>>  - ... and 69 more: https://git.openjdk.org/jdk/compare/c266800a...20e92608
>
> src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 416:
> 
>> 414: 
>> 415:     // mark contains the tagged ObjectMonitor*.
>> 416:     const Register monitor = mark;
> 
> I wouldn't do this. Making `monitor` a register alias for `mark` is at best confusing and at worst dangerous. As a general rule, aliased register names have caused many bugs.

I have been trying to think of alternatives which improve this situation. 

Is it the fact that monitor alias mark, or any of the aliases used for the in-parameters that is confusing?
If it is the first do you see the following as a solution:
```c++
    const Register monitor = t1;
    const uintptr_t monitor_tag = markWord::monitor_value;

    // Untag the monitor.
    sub(monitor, mark, monitor_tag);

and for the lock case
```c++
    const Register tagged_monitor = t1;
    mov(tagged_monitor, mark); 

where the mov will emit no instruction as they are the same register.
(These two variants seem somewhat equivalent to me, but it was not clear from your comment if this is the issue)

If it is the second, I feel like the best compromise would to still alias register via named variables, but give them names that show their origin (so you do not need a language server or scroll/grep to get the contextual information). 
E.g.
```c++
    const Register t1_monitor = t1;
    const uintptr_t monitor_tag = markWord::monitor_value;

    // Untag the monitor.
    sub(t1_monitor, t1_mark, monitor_tag);


I would like to hear if I have misunderstood something and / or if you have another proposed solution to this issue.

This seem to be the final issue blocking integration.

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

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