RFR: 8344010: RISC-V: Zacas do not work with LW locking

Robbin Ehn rehn at openjdk.org
Wed Nov 20 06:28:15 UTC 2024


On Tue, 19 Nov 2024 20:55:17 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi all, please consider.
>> 
>> Light weight locking fails:
>> - We need to add cas acquire.
>> - Register _result_ may shadow _new_val_ (same register).
>>   (NOTE this second item can effect many other cases, unclear)
>> 
>> As the code becomes much cleaner by calling amocas_d/w directly I removed the aliases.
>> Which fixes the first issue with cas acquire.
>> 
>> By using t0 instead of _result_ we fix the other issue.
>> 
>> This is a short bugfix, there are so many dragons here that I do not want to address them while fixing the bug.
>> There are also several performance optimizations we can do here, specially for LR/SC case.
>> So I'll do a couple of more iterations of this code in seperate PR's.
>> 
>> Testing a bunch of local cherry-picked tests which failed.
>> I'll start tier1 over the weekend.
>> 
>> Thanks, Robbin
>
> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 3488:
> 
>> 3486:     } else {
>> 3487:       mv(t0, expected);
>> 3488:       atomic_cas(t0, new_val, addr, size, acquire, release);
> 
> Not sure if I understand this correctly.
> Could it be better to tell if `result` == `new_val`? If false, then we don't need the extra `mv`.

The "mv()" check that for you, so an e.g. mv(a0, a0) will never emit an instruction.

OT NOTE:
If you are in an InCompressableRegion this will still not emit an instruction so your offsets may be wrong.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22149#discussion_r1849621841


More information about the hotspot-dev mailing list