On Tue, 28 Mar 2023 00:17:21 GMT, Dean Long <dl...@openjdk.org> wrote:
>> Roman Kennke has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Ensure safepoint when processing lock-stack
>
> src/hotspot/cpu/aarch64/aarch64.ad line 3844:
>
>> 3842:
>> 3843: // Indicate success on completion.
>> 3844: __ cmp(oop, oop); // Force ZF=1 to indicate success.
>
> It looks like `fast_lock` already sets ZF=1 on success/fall-through. Why not
> document this as part of the interface, then this `cmp` will be redundant?
Indeed. I was assuming that the instructions that follow the CAS might be
affecting the NZCV flags (like they do on x86), but apparently they don't. Very
nice. I am removing the instruction here and following fast_unlock().
> src/hotspot/share/interpreter/interpreterRuntime.cpp line 740:
>
>> 738: if (!UseHeavyMonitors && UseFastLocking) {
>> 739: // This is a hack to get around the limitation of registers in
>> x86_32. We really
>> 740: // send an oopDesc* instead of a BasicObjectLock*.
>
> I don't understand what this is referring to. Trying to avoid passing an
> extra argument?
I updated the comment in an earlier commit to say:
// TODO: We accept elem as void* to workaround a limitation of registers in
x86_32. Interpreter
// code is really sending an oopDesc* here.
// The problem is that we would need to preserve the register that holds
the BasicObjectLock,
// but we are using that register to hold the thread. We don't have enough
registers to
// also keep the BasicObjectLock, but we don't really need it anyway, we
only need
// the object. See also InterpreterMacroAssembler::lock_object().
// As soon as traditional stack-locking goes away we can change elem to be
oopDesc*
// (also in monitorexit below).
I hope that is clearer.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1153268963
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1153270625
