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

[Robbin Ehn]

On Mon, 18 Nov 2024 02:57:39 GMT, Fei Yang <fyang 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 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.

The reason to move it after is readability.
By having the zacas code before the asserts you think the implied contract the asserts states do not apply to zacas case. Meaning that, at least I, read the code as addr, expected, new_val may be t0 when UseZacas is true.

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

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