Re: [External] Re: [PATCH] riscv/futex: sign extend compare value in atomic cmpxchg

[yunhui cui]

Hi,

On Fri, Feb 14, 2025 at 2:31 AM <patchwork-bot+linux-riscv@xxxxxxxxxx> wrote:
>
> Hello:
>
> This patch was applied to riscv/linux.git (fixes)
> by Palmer Dabbelt <palmer@xxxxxxxxxxxx>:
>
> On Mon, 03 Feb 2025 11:06:00 +0100 you wrote:
> > Make sure the compare value in the lr/sc loop is sign extended to match
> > what lr.w does.  Fortunately, due to the compiler keeping the register
> > contents sign extended anyway the lack of the explicit extension didn't
> > result in wrong code so far, but this cannot be relied upon.
> >
> > Fixes: b90edb33010b ("RISC-V: Add futex support.")
> > Signed-off-by: Andreas Schwab <schwab@xxxxxxx>
> >
> > [...]
>
> Here is the summary with links:
>   - riscv/futex: sign extend compare value in atomic cmpxchg
>     https://git.kernel.org/riscv/c/5c238584bce5
>
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
>
>

I applied this patch, but it doesn't seem to take effect. There is no
sign extension for a2 in the assembly code. What did I miss?
GCC version >= 13.

ffffffff800e87e0 <futex_atomic_cmpxchg_inatomic>:
ffffffff800e87e0: 1141                addi sp,sp,-16
ffffffff800e87e2: e422                sd s0,8(sp)
ffffffff800e87e4: 2bf01793          bseti a5,zero,0x3f
ffffffff800e87e8: 0800                addi s0,sp,16
ffffffff800e87ea: 17ed                addi a5,a5,-5
ffffffff800e87ec: 00b7f663          bgeu a5,a1,ffffffff800e87f8
<futex_atomic_cmpxchg_inatomic+0x18>
ffffffff800e87f0: 6422                ld s0,8(sp)
ffffffff800e87f2: 5549                li a0,-14
ffffffff800e87f4: 0141                addi sp,sp,16
ffffffff800e87f6: 8082                ret
ffffffff800e87f8: 872a                mv a4,a0
ffffffff800e87fa: 00040837          lui a6,0x40
ffffffff800e87fe: 10082073          csrs sstatus,a6
ffffffff800e8802: 4781                li a5,0
ffffffff800e8804: 1605a8af          lr.w.aqrl a7,(a1)
ffffffff800e8808: 00c89563          bne a7,a2,ffffffff800e8812
<futex_atomic_cmpxchg_inatomic+0x32>
ffffffff800e880c: 1ed5a52f          sc.w.aqrl a0,a3,(a1)
ffffffff800e8810: f975                bnez a0,ffffffff800e8804
<futex_atomic_cmpxchg_inatomic+0x24>
ffffffff800e8812: 0007851b          sext.w a0,a5
ffffffff800e8816: 10083073          csrc sstatus,a6
ffffffff800e881a: 01172023          sw a7,0(a4)
ffffffff800e881e: 6422                ld s0,8(sp)
ffffffff800e8820: 0141                addi sp,sp,16
ffffffff800e8822: 8082                ret

Should we do it like this:
__asm__ __volatile__ (
" sext.w %[ov], %[ov] \n"
...


Thanks,
Yunhui