Re: [PATCH] RISC-V: Fix __atomic_compare_exchange with 32 bit value on RV64

[Patrick O'Neill <patrick@rivosinc.com>]

On 2/28/24 07:02, Palmer Dabbelt wrote:
> On Wed, 28 Feb 2024 06:57:53 PST (-0800), jeffreyalaw@gmail.com wrote:
>>
>>
>> On 2/28/24 05:23, Kito Cheng wrote:
>>> atomic_compare_and_swapsi will use lr.w and sc.w to do the atomic 
>>> operation on
>>> RV64, however lr.w is doing sign extend to DI and compare 
>>> instruction only have
>>> DI mode on RV64, so the expected value should be sign extend before 
>>> compare as
>>> well, so that we can get right compare result.
>>>
>>> gcc/ChangeLog:
>>>
>>>     PR target/114130
>>>     * config/riscv/sync.md (atomic_compare_and_swap<mode>): Sign
>>>     extend the expected value if needed.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     * gcc.target/riscv/pr114130.c: New.
>> Nearly rejected this as I think the description was a bit ambiguous and
>> I thought you were extending the result of the lr.w.  But it's actually
>> the other value you're ensuring gets properly extended.
>
> I had the same response, but after reading it I'm not quite sure how 
> to say it better.
>
>> OK.
>
> I was looking at the code to try and ask if we have the same bug for 
> the short inline CAS routines, but I've got to run to some meetings...

I don't think subword AMO CAS is impacted.

As part of the CAS we mask both the expected value [2] and the retrieved 
value[1] before comparing.

- Patrick

[1]: 
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/config/riscv/sync.md;h=54bb0a66518ae353fa4ed640339213bf5da6682c;hb=refs/heads/master#l495
[2]: 
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/config/riscv/sync.md;h=54bb0a66518ae353fa4ed640339213bf5da6682c;hb=refs/heads/master#l459

>
>>
>> Jeff