RFR: 8338383: Implementation of Synchronize Virtual Threads without Pinning [v3]

Andrew Haley aph at openjdk.org
Tue Oct 22 15:58:24 UTC 2024


On Tue, 22 Oct 2024 15:48:43 GMT, Andrew Haley <aph at 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


More information about the graal-dev mailing list