On Tue, 22 Oct 2024 15:48:43 GMT, Andrew Haley <a...@openjdk.org> wrote:
>> src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 60: >> >>> 58: >>> 59: assert(LockingMode != LM_LIGHTWEIGHT, "lightweight locking should use >>> fast_lock_lightweight"); >>> 60: assert_different_registers(oop, box, tmp, disp_hdr, rscratch2); >> >> Historically, silently using `rscratch1` and `rscratch2` in these macros has >> sometimes turned out to be a mistake. >> Please consider making `rscratch2` an additional argument to `fast_lock`, so >> that it's explicit in the caller. It won't make any difference to the >> generated code, but it might help readbility. > > Note also that `inc_held_monitor_count` clobbers `rscratch2`. That might be > worth a comment at the call site. > I guess `inc_held_monitor_count` is so hot that we can't push and pop scratch > registers, in which case it'd clobber nothing. > Historically, silently using `rscratch1` and `rscratch2` in these macros has > sometimes turned out to be a mistake. Please consider making `rscratch2` an > additional argument to `fast_lock`, so that it's explicit in the caller. It > won't make any difference to the generated code, but it might help readbility. Hmm, forget that. It's rather tricky code, that's true, but I think we're OK. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1810998545
