patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Jerin Jacob <jerinjacobk@gmail.com>
To: Ruifeng Wang <Ruifeng.Wang@arm.com>
Cc: Joyce Kong <Joyce.Kong@arm.com>,
	"jerinj@marvell.com" <jerinj@marvell.com>,
	"david.marchand@redhat.com" <david.marchand@redhat.com>,
	 Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
	"dev@dpdk.org" <dev@dpdk.org>, nd <nd@arm.com>,
	 "stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v1] eal/arm: fix gcc build for 128-bit atomic compare exchange
Date: Wed, 27 Jan 2021 15:26:39 +0530	[thread overview]
Message-ID: <CALBAE1ObbmvFNhS0KTGUeWFhOeeWAc-WbEO5d5=K-4yH5fYtag@mail.gmail.com> (raw)
In-Reply-To: <HE1PR0802MB23450A4652C5664A2383D6799EBB0@HE1PR0802MB2345.eurprd08.prod.outlook.com>

On Wed, Jan 27, 2021 at 9:06 AM Ruifeng Wang <Ruifeng.Wang@arm.com> wrote:
>
> > -----Original Message-----
> > From: Joyce Kong <joyce.kong@arm.com>
> > Sent: Friday, January 15, 2021 5:58 PM
> > To: jerinj@marvell.com; david.marchand@redhat.com; Ruifeng Wang
> > <Ruifeng.Wang@arm.com>; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>
> > Cc: dev@dpdk.org; nd <nd@arm.com>; stable@dpdk.org
> > Subject: [PATCH v1] eal/arm: fix gcc build for 128-bit atomic compare
> > exchange
> >
> > Compiling with "meson build -Dbuildtype=debug --cross-file
> > config/arm/arm64_thunderx2_linux_gcc" shows the warnings "function
>
> Issue can be reproduced with the posted command. But it is not specific to ThunderX2 platform.
> It is reproducible on any platform that has LSE extension when building with 'buildtype=debug'.
>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>


Acked-by: Jerin Jacob <jerinj@marvell.com>


>
> > returns an aggregate [-Waggregate-return]":
> > ../../dpdk/lib/librte_eal/arm/include/rte_atomic_64.h: In function
> > ‘__cas_128_relaxed’:
> > ../../dpdk/lib/librte_eal/arm/include/rte_atomic_64.h:81:20:
> > error: function returns an aggregate [-Werror=aggregate-return]
> > __ATOMIC128_CAS_OP(__cas_128_relaxed, "casp")
> >                     ^~~~~~~~~~~~~~~~~
> >
> > Fix the compiling issue by defining __ATOMIC128_CAS_OP as a void function
> > and passing the address pointer into it.
> >
> > Fixes: 7e2c3e17fe2c ("eal/arm64: add 128-bit atomic compare exchange")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> > ---
> >  lib/librte_eal/arm/include/rte_atomic_64.h | 28 +++++++++++-----------
> >  1 file changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/lib/librte_eal/arm/include/rte_atomic_64.h
> > b/lib/librte_eal/arm/include/rte_atomic_64.h
> > index 467d32a45..fa6f334c0 100644
> > --- a/lib/librte_eal/arm/include/rte_atomic_64.h
> > +++ b/lib/librte_eal/arm/include/rte_atomic_64.h
> > @@ -53,15 +53,15 @@ rte_atomic_thread_fence(int memorder)  #endif
> >
> >  #define __ATOMIC128_CAS_OP(cas_op_name, op_string)                          \
> > -static __rte_noinline rte_int128_t                                          \
> > -cas_op_name(rte_int128_t *dst, rte_int128_t old, rte_int128_t updated)
> > \
> > +static __rte_noinline void                                                  \
> > +cas_op_name(rte_int128_t *dst, rte_int128_t *old, rte_int128_t updated)
> > \
> >  {                                                                           \
> >       /* caspX instructions register pair must start from even-numbered
> >        * register at operand 1.
> >        * So, specify registers for local variables here.
> >        */                                                                 \
> > -     register uint64_t x0 __asm("x0") = (uint64_t)old.val[0];            \
> > -     register uint64_t x1 __asm("x1") = (uint64_t)old.val[1];            \
> > +     register uint64_t x0 __asm("x0") = (uint64_t)old->val[0];           \
> > +     register uint64_t x1 __asm("x1") = (uint64_t)old->val[1];           \
> >       register uint64_t x2 __asm("x2") = (uint64_t)updated.val[0];        \
> >       register uint64_t x3 __asm("x3") = (uint64_t)updated.val[1];        \
> >       asm volatile(                                                       \
> > @@ -73,9 +73,8 @@ cas_op_name(rte_int128_t *dst, rte_int128_t old,
> > rte_int128_t updated)      \
> >               [upd1] "r" (x3),                                            \
> >               [dst] "r" (dst)                                             \
> >               : "memory");                                                \
> > -     old.val[0] = x0;                                                    \
> > -     old.val[1] = x1;                                                    \
> > -     return old;                                                         \
> > +     old->val[0] = x0;                                                   \
> > +     old->val[1] = x1;                                                   \
> >  }
> >
> >  __ATOMIC128_CAS_OP(__cas_128_relaxed, "casp") @@ -113,13 +112,14
> > @@ rte_atomic128_cmp_exchange(rte_int128_t *dst, rte_int128_t *exp,
> >
> >  #if defined(__ARM_FEATURE_ATOMICS) ||
> > defined(RTE_ARM_FEATURE_ATOMICS)
> >       if (success == __ATOMIC_RELAXED)
> > -             old = __cas_128_relaxed(dst, expected, desired);
> > +             __cas_128_relaxed(dst, exp, desired);
> >       else if (success == __ATOMIC_ACQUIRE)
> > -             old = __cas_128_acquire(dst, expected, desired);
> > +             __cas_128_acquire(dst, exp, desired);
> >       else if (success == __ATOMIC_RELEASE)
> > -             old = __cas_128_release(dst, expected, desired);
> > +             __cas_128_release(dst, exp, desired);
> >       else
> > -             old = __cas_128_acq_rel(dst, expected, desired);
> > +             __cas_128_acq_rel(dst, exp, desired);
> > +     old = *exp;
> >  #else
> >  #define __HAS_ACQ(mo) ((mo) != __ATOMIC_RELAXED && (mo) !=
> > __ATOMIC_RELEASE)  #define __HAS_RLS(mo) ((mo) ==
> > __ATOMIC_RELEASE || (mo) == __ATOMIC_ACQ_REL || \ @@ -183,12
> > +183,12 @@ rte_atomic128_cmp_exchange(rte_int128_t *dst, rte_int128_t
> > *exp,  #undef __STORE_128
> >
> >       } while (unlikely(ret));
> > -#endif
> >
> > -     /* Unconditionally updating expected removes an 'if' statement.
> > -      * expected should already be in register if not in the cache.
> > +     /* Unconditionally updating the value of exp removes an 'if'
> > statement.
> > +      * The value of exp should already be in register if not in the cache.
> >        */
> >       *exp = old;
> > +#endif
> >
> >       return (old.int128 == expected.int128);  }
> > --
> > 2.30.0
>

  reply	other threads:[~2021-01-27  9:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-15  9:58 [dpdk-stable] " Joyce Kong
2021-01-15 15:55 ` David Marchand
2021-01-21  9:36   ` Joyce Kong
2021-01-27  3:36 ` Ruifeng Wang
2021-01-27  9:56   ` Jerin Jacob [this message]
2021-01-27 10:22 ` David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALBAE1ObbmvFNhS0KTGUeWFhOeeWAc-WbEO5d5=K-4yH5fYtag@mail.gmail.com' \
    --to=jerinjacobk@gmail.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=Joyce.Kong@arm.com \
    --cc=Ruifeng.Wang@arm.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --cc=nd@arm.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).