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

Fei Yang fyang at openjdk.org
Mon Nov 18 03:02:53 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

Good catch. Thanks for carrying out the test. Looks good modulo one minor question.

src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 3533:

> 3531:     cmpxchg(addr, expected, new_val, size, acquire, release, result, true);
> 3532:     return;
> 3533:   }

Question: Should we move the code after the three assertions? I see same assertions are there in `MacroAssembler::cmpxchg` as well. Even though they are the same for now, I assume they are targeting different code and are submit to change in the future.

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

PR Review: https://git.openjdk.org/jdk/pull/22149#pullrequestreview-2441399914
PR Review Comment: https://git.openjdk.org/jdk/pull/22149#discussion_r1845788735


More information about the hotspot-dev mailing list