diff mbox series

[RFC] eal/arm: remove CASP constraints for GCC

Message ID 20211004100304.13602-1-pbhagavatula@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series [RFC] eal/arm: remove CASP constraints for GCC |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Pavan Nikhilesh Bhagavatula Oct. 4, 2021, 10:03 a.m. UTC 
  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(+)

Comments

Ruifeng Wang Oct. 18, 2021, 6:39 a.m. UTC | #1 
> -----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
diff mbox series

Patch

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                                              \
+		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")