Re: [dpdk-dev] [RFC] eal/arm: remove CASP constraints for GCC

[Ruifeng Wang <Ruifeng.Wang@arm.com>]

> -----Original Message-----
> From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> Sent: Monday, October 4, 2021 6:03 PM
> To: jerinj@marvell.com; Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@marvell.com>
> Subject: [dpdk-dev] [RFC] eal/arm: remove CASP constraints for GCC
> 
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> GCC now assigns even register pairs for CASP, the fix has also been
> backported to all stable releases of older GCC versions.
> Removing the manual register allocation allows GCC to inline the functions
> and pick optimal registers for performing CASP.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>  lib/eal/arm/include/rte_atomic_64.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/lib/eal/arm/include/rte_atomic_64.h
> b/lib/eal/arm/include/rte_atomic_64.h
> index fa6f334c0d..f6f31ae777 100644
> --- a/lib/eal/arm/include/rte_atomic_64.h
> +++ b/lib/eal/arm/include/rte_atomic_64.h
> @@ -52,6 +52,7 @@ rte_atomic_thread_fence(int memorder)
>  #define __LSE_PREAMBLE	""
>  #endif
> 
> +#if defined(__clang__)
>  #define __ATOMIC128_CAS_OP(cas_op_name, op_string)                          \
>  static __rte_noinline void                                                  \
>  cas_op_name(rte_int128_t *dst, rte_int128_t *old, rte_int128_t updated)
> \
> @@ -76,6 +77,19 @@ cas_op_name(rte_int128_t *dst, rte_int128_t *old,
> rte_int128_t updated)     \
>  	old->val[0] = x0;                                                   \
>  	old->val[1] = x1;                                                   \
>  }
> +#else
> +#define __ATOMIC128_CAS_OP(cas_op_name, op_string)                          \
> +static __rte_always_inline void                                             \
> +cas_op_name(rte_int128_t *dst, rte_int128_t *old, rte_int128_t updated)
> \
> +{                                                                           \
> +	asm volatile(                                                       \
> +		__LSE_PREAMBLE                                              \
Change looks good.

One minor comment, gcc doesn't need this PREAMBLE.

Thanks,
Ruifeng
> +		op_string " %[old], %H[old], %[upd], %H[upd], [%[dst]]"     \
> +		: [old] "+r"(old->int128)                                   \
> +		: [upd] "r"(updated.int128), [dst] "r"(dst)                 \
> +		: "memory");                                                \
> +}
> +#endif
> 
>  __ATOMIC128_CAS_OP(__cas_128_relaxed, "casp")
> __ATOMIC128_CAS_OP(__cas_128_acquire, "caspa")
> --
> 2.17.1