RISC-V: Execute a PAUSE hint in spin loops

Message ID 20240418215858.4151969-1-christoph.muellner@vrull.eu
State Accepted
Delegated to: Palmer Dabbelt
Headers
Series RISC-V: Execute a PAUSE hint in spin loops |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed

Commit Message

Christoph Müllner April 18, 2024, 9:58 p.m. UTC
  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(+)
  

Comments

Palmer Dabbelt April 19, 2024, 1:03 a.m. UTC | #1
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!
  

Patch

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 */