[RFC] eal: use same atomic intrinsics for gcc and clang

  The size generic atomic intrinsics generate the same
code as the size specific intrinsics for gcc. Use size
generic intrinsics for both gcc and clang.

Fixes: 7bdccb93078e ("eal: fix ARM build with clang")
Cc: stable@dpdk.org
Cc: pbhagavatula@marvell.com

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/eal/include/generic/rte_atomic.h | 12 ------------
 1 file changed, 12 deletions(-)
  

> From: Honnappa Nagarahalli [mailto:honnappa.nagarahalli@arm.com]
> Sent: Saturday, 11 February 2023 02.56
> 
> The size generic atomic intrinsics generate the same
> code as the size specific intrinsics for gcc. Use size
> generic intrinsics for both gcc and clang.
> 
> Fixes: 7bdccb93078e ("eal: fix ARM build with clang")
> Cc: stable@dpdk.org
> Cc: pbhagavatula@marvell.com
> 
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---

Acked-by: Morten Brørup <mb@smartsharesystems.com>
  

On Fri, Feb 10, 2023 at 07:56:22PM -0600, Honnappa Nagarahalli wrote:
> The size generic atomic intrinsics generate the same
> code as the size specific intrinsics for gcc. Use size
> generic intrinsics for both gcc and clang.
> 
> Fixes: 7bdccb93078e ("eal: fix ARM build with clang")
> Cc: stable@dpdk.org
> Cc: pbhagavatula@marvell.com
> 
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

so first of all thanks for fixing this up. it was confusing me why it
was being conditionally compiled (there was no documented rationale).

i do have a slight concern that perhaps the reason this was done was
because __atomic_exchange_n on some gcc versions or for some specific
target processor was generating less than desirable code but if nobody
else has this concern i won't get in the way since it simplifies things.

thanks!

> ---
>  lib/eal/include/generic/rte_atomic.h | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/lib/eal/include/generic/rte_atomic.h b/lib/eal/include/generic/rte_atomic.h
> index f5c49a9870..234b268b91 100644
> --- a/lib/eal/include/generic/rte_atomic.h
> +++ b/lib/eal/include/generic/rte_atomic.h
> @@ -176,11 +176,7 @@ rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val);
>  static inline uint16_t
>  rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val)
>  {
> -#if defined(__clang__)
>  	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);
> -#else
> -	return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST);
> -#endif
>  }
>  #endif
>  
> @@ -459,11 +455,7 @@ rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val);
>  static inline uint32_t
>  rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val)
>  {
> -#if defined(__clang__)
>  	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);
> -#else
> -	return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
> -#endif
>  }
>  #endif
>  
> @@ -741,11 +733,7 @@ rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val);
>  static inline uint64_t
>  rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val)
>  {
> -#if defined(__clang__)
>  	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST);
> -#else
> -	return __atomic_exchange_8(dst, val, __ATOMIC_SEQ_CST);
> -#endif
>  }
>  #endif
>  
> -- 
> 2.25.1
  

<snip>

> 
> On Fri, Feb 10, 2023 at 07:56:22PM -0600, Honnappa Nagarahalli wrote:
> > The size generic atomic intrinsics generate the same code as the size
> > specific intrinsics for gcc. Use size generic intrinsics for both gcc
> > and clang.
> >
> > Fixes: 7bdccb93078e ("eal: fix ARM build with clang")
> > Cc: stable@dpdk.org
> > Cc: pbhagavatula@marvell.com
> >
> > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> 
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> 
> so first of all thanks for fixing this up. it was confusing me why it was being
> conditionally compiled (there was no documented rationale).
> 
> i do have a slight concern that perhaps the reason this was done was because
> __atomic_exchange_n on some gcc versions or for some specific target
> processor was generating less than desirable code but if nobody else has this
> concern i won't get in the way since it simplifies things.
Agree, I could not find a rationale from the commit logs. However, I did some tests on Godbolt and the latest compilers generated the same code for both the variants for x86 and ARM.
I think the initial author was Stephen, may be he has some comments.

> 
> thanks!
> 
> > ---
> >  lib/eal/include/generic/rte_atomic.h | 12 ------------
> >  1 file changed, 12 deletions(-)
> >
> > diff --git a/lib/eal/include/generic/rte_atomic.h
> > b/lib/eal/include/generic/rte_atomic.h
> > index f5c49a9870..234b268b91 100644
> > --- a/lib/eal/include/generic/rte_atomic.h
> > +++ b/lib/eal/include/generic/rte_atomic.h
> > @@ -176,11 +176,7 @@ rte_atomic16_exchange(volatile uint16_t *dst,
> > uint16_t val);  static inline uint16_t  rte_atomic16_exchange(volatile
> > uint16_t *dst, uint16_t val)  { -#if defined(__clang__)
> >  	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST); -#else
> > -	return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST);
> > -#endif
> >  }
> >  #endif
> >
> > @@ -459,11 +455,7 @@ rte_atomic32_exchange(volatile uint32_t *dst,
> > uint32_t val);  static inline uint32_t  rte_atomic32_exchange(volatile
> > uint32_t *dst, uint32_t val)  { -#if defined(__clang__)
> >  	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST); -#else
> > -	return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
> > -#endif
> >  }
> >  #endif
> >
> > @@ -741,11 +733,7 @@ rte_atomic64_exchange(volatile uint64_t *dst,
> > uint64_t val);  static inline uint64_t  rte_atomic64_exchange(volatile
> > uint64_t *dst, uint64_t val)  { -#if defined(__clang__)
> >  	return __atomic_exchange_n(dst, val, __ATOMIC_SEQ_CST); -#else
> > -	return __atomic_exchange_8(dst, val, __ATOMIC_SEQ_CST);
> > -#endif
> >  }
> >  #endif
> >
> > --
> > 2.25.1
  

> -----Original Message-----
> From: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Sent: Saturday, February 11, 2023 9:56 AM
> To: roretzla@linux.microsoft.com; bruce.richardson@intel.com; mb@smartsharesystems.com
> Cc: dev@dpdk.org; Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; jerinj@marvell.com; nd <nd@arm.com>; stable@dpdk.org;
> pbhagavatula@marvell.com
> Subject: [RFC] eal: use same atomic intrinsics for gcc and clang
> 
> The size generic atomic intrinsics generate the same code as the size specific intrinsics
> for gcc. Use size generic intrinsics for both gcc and clang.
> 
> Fixes: 7bdccb93078e ("eal: fix ARM build with clang")
> Cc: stable@dpdk.org
> Cc: pbhagavatula@marvell.com
> 
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>

Thanks.