RFR: 8319901: Recursive lightweight locking: ppc64le implementation

[Axel Boldt-Christmas]

On Wed, 21 Feb 2024 04:48:30 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

> Thanks a lot! LGTM.
> 
> Regarding aarch64: Should we file a JBS issue for the usage of `box`?
> 
> https://github.com/openjdk/jdk/blob/d31fd78d963d5d103b1b1bf66ae0bdbe4be2b790/src/hotspot/cpu/aarch64/aarch64.ad#L16470
> 
> 
> has no kill effect for `box`, but `C2_MacroAssembler::fast_lock_lightweight` kills it. It may be never used, but it's a trap someone might run into. x86_64 uses `USE_KILL box`.
> I think that it currently works, but I'd prefer to use a temp register and hopefully remove the `box` completely at some point of time. It could also be cleaned up together with other changes if you prefer that.

This came up in the review as well. The issue with `USE_KILL` and `KILL` is that they require a bound register to use. So we must restrict the box to a specific real register. (Works on x86 because it box is bound to `RAX`/`EAX`).

Should create a JBS issue for this. And maybe it is to dangerous to leave it as is until we can create a C2 Node that does not require the box.

The only potential fix I see is to actually bind the register.  i.e.

diff --git a/src/hotspot/cpu/aarch64/aarch64.ad b/src/hotspot/cpu/aarch64/aarch64.ad
index a07ff041c48..97d7898ceba 100644
--- a/src/hotspot/cpu/aarch64/aarch64.ad
+++ b/src/hotspot/cpu/aarch64/aarch64.ad
@@ -16463,14 +16463,14 @@ instruct cmpFastUnlock(rFlagsReg cr, iRegP object, iRegP box, iRegPNoSp tmp, iRe
   ins_pipe(pipe_serial);
 %}
 
-instruct cmpFastLockLightweight(rFlagsReg cr, iRegP object, iRegP box, iRegPNoSp tmp, iRegPNoSp tmp2)
+instruct cmpFastLockLightweight(rFlagsReg cr, iRegP object, iRegP_R0 box, iRegPNoSp tmp, iRegPNoSp tmp2)
 %{
   predicate(LockingMode == LM_LIGHTWEIGHT);
   match(Set cr (FastLock object box));
-  effect(TEMP tmp, TEMP tmp2);
+  effect(TEMP tmp, TEMP tmp2, USE_KILL box);
 
   ins_cost(5 * INSN_COST);
-  format %{ "fastlock $object,$box\t! kills $tmp,$tmp2" %}
+  format %{ "fastlock $object,$box\t! kills $tmp,$tmp2,$box" %}
 
   ins_encode %{
     __ fast_lock_lightweight($object$$Register, $box$$Register, $tmp$$Register, $tmp2$$Register);
@@ -16479,14 +16479,14 @@ instruct cmpFastLockLightweight(rFlagsReg cr, iRegP object, iRegP box, iRegPNoSp
   ins_pipe(pipe_serial);
 %}
 
-instruct cmpFastUnlockLightweight(rFlagsReg cr, iRegP object, iRegP box, iRegPNoSp tmp, iRegPNoSp tmp2)
+instruct cmpFastUnlockLightweight(rFlagsReg cr, iRegP object, iRegP_R0 box, iRegPNoSp tmp, iRegPNoSp tmp2)
 %{
   predicate(LockingMode == LM_LIGHTWEIGHT);
   match(Set cr (FastUnlock object box));
-  effect(TEMP tmp, TEMP tmp2);
+  effect(TEMP tmp, TEMP tmp2, USE_KILL box);
 
   ins_cost(5 * INSN_COST);
-  format %{ "fastunlock $object,$box\t! kills $tmp, $tmp2" %}
+  format %{ "fastunlock $object,$box\t! kills $tmp, $tmp2, $box" %}
 
   ins_encode %{
     __ fast_unlock_lightweight($object$$Register, $box$$Register, $tmp$$Register, $tmp2$$Register);


Maybe someone with better knowledge of the ADLC knows a better solution.

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

PR Comment: https://git.openjdk.org/jdk/pull/16611#issuecomment-1956024759