Re: [PATCH] RISC-V: Execute a PAUSE hint in spin loops

[Palmer Dabbelt <palmer@dabbelt.com>]

On Thu, 18 Apr 2024 14:58:58 PDT (-0700), christoph.muellner@vrull.eu wrote:
> The atomic_spin_nop() macro can be used to run arch-specific
> code in the body of a spin loop to potentially improve efficiency.
> RISC-V's Zihintpause extension includes a PAUSE instruction for
> this use-case, which is encoded as a HINT, which means that it
> behaves like a NOP on systems that don't implement Zihintpause.
>
> Binutils supports Zihintpause since 2.36, so this patch uses
> the ".insn" directive to keep the code compatible with older
> toolchains.
>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> ---
>  sysdeps/unix/sysv/linux/riscv/atomic-machine.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/sysdeps/unix/sysv/linux/riscv/atomic-machine.h b/sysdeps/unix/sysv/linux/riscv/atomic-machine.h
> index c1c9d949a0..90283d9746 100644
> --- a/sysdeps/unix/sysv/linux/riscv/atomic-machine.h
> +++ b/sysdeps/unix/sysv/linux/riscv/atomic-machine.h
> @@ -178,4 +178,7 @@
>  # error "ISAs that do not subsume the A extension are not supported"
>  #endif /* !__riscv_atomic */
>
> +/* Execute a PAUSE hint when spinning.  */
> +#define atomic_spin_nop() __asm(".insn i 0x0f, 0, x0, x0, 0x010")
> +
>  #endif /* bits/atomic.h */

I think this one should be pretty non-controversial: the whole point of 
PAUSE is for this sort of spin loop, so I don't think we need any 
benchmarks to justify using it.  So

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

IIRC there's some tunables for spin counts already, so hopefully that's 
enough to cover us for now.  I guess we'll found out when people have 
hardware...

Thanks!