RFR: 8344010: RISC-V: Zacas do not work with LW locking
Hamlin Li
mli at openjdk.org
Tue Nov 19 20:57:50 UTC 2024
On Fri, 15 Nov 2024 13:01:37 GMT, Robbin Ehn <rehn 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
Nice catch and cleanup, looks much better!
Just one minor comment.
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`.
-------------
PR Review: https://git.openjdk.org/jdk/pull/22149#pullrequestreview-2446635561
PR Review Comment: https://git.openjdk.org/jdk/pull/22149#discussion_r1849044858
More information about the hotspot-dev
mailing list