RFR: 8296170: Refactor stack-locking path in C2_MacroAssembler::fast_unlock() [v2]

[Tobias Hartmann]

On Thu, 10 Nov 2022 15:13:48 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> The code in C2_MacroAssembler::fast_unlock() has several (minor) issues: 
>>  - The stack-locking path for x86_32 is not under UseHeavyMonitors - it would be executed even when stack-locking is disabled. 
>>  - The stack-locking paths are the same for x86_32 and x86_64 - they can be merged into a common path. 
>>  - In x86_32 path, we call get_thread(boxReg) which is totally bogus because we clear boxReg right afterwards with xorptr(boxReg, boxReg). 
>>  - In x86_32 path, the CheckSucc label is identical to the DONE label, and in-fact CheckSucc is only ever really used in the x86_64 path and can be moved there.
>> 
>> Testing:
>>  - [x] tier1 (x86_32, x86_64)
>>  - [x] tier2 (x86_32, x86_64)
>
> Roman Kennke 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 two additional commits since the last revision:
> 
>  - Merge remote-tracking branch 'upstream/master' into JDK-8296170
>  - 8296170: Refactor stack-locking path in C2_MacroAssembler::fast_unlock()

src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 928:

> 926:   }
> 927: 
> 928:   // TODO: Comment still valid?

We have the same comment here:
https://github.com/openjdk/jdk/blob/12e76cbc725ff87577e2ef23267590eae37a82d1/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp#L707

And it seems to be there from the beginning:
https://hg.openjdk.java.net/jdk/jdk/annotate/74aaad871363/hotspot/src/cpu/x86/vm/x86_32.ad#l3441

I think instead of adding a `TODO`, we should either completely remove the comments or file a follow-up enhancement to investigate.

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

PR: https://git.openjdk.org/jdk/pull/10936