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

yunhui cui
2025-02-14T04:11:18Z

Hi,

On Fri, Feb 14, 2025 at 2:31 AM <patchwork-bot+linux-riscv@kernel.org> wrote:
>
> Hello:
>
> This patch was applied to riscv/linux.git (fixes)
> by Palmer Dabbelt <palmer@rivosinc.com>:
>
> 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@suse.de>
> >
> > [...]
>
> 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