[v1,4/5] spinlock: move the implementation to arm specific file

  remove the hard code #ifdef RTE_FORCE_INTRINSICS, move the implementation
to the arm specific file, x86 and POWER have their own implementations.

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Ruifeng Wang <Ruifeng.Wang@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
---
 .../common/include/arch/arm/rte_spinlock.h         | 20 +++++++++++++++++
 .../common/include/generic/rte_spinlock.h          | 26 ----------------------
 2 files changed, 20 insertions(+), 26 deletions(-)
  

On Thu, Dec 20, 2018 at 11:44 AM Gavin Hu <gavin.hu@arm.com> wrote:

> remove the hard code #ifdef RTE_FORCE_INTRINSICS, move the implementation
> to the arm specific file, x86 and POWER have their own implementations.
>

No, x86 and ppc define their own implementation when _not_ having
RTE_FORCE_INTRINSICS.
  

On Thu, Dec 20, 2018 at 1:47 PM David Marchand <david.marchand@redhat.com>
wrote:

> On Thu, Dec 20, 2018 at 11:44 AM Gavin Hu <gavin.hu@arm.com> wrote:
>
>> remove the hard code #ifdef RTE_FORCE_INTRINSICS, move the implementation
>> to the arm specific file, x86 and POWER have their own implementations.
>>
>
> No, x86 and ppc define their own implementation when _not_ having
> RTE_FORCE_INTRINSICS.
>

Actually, I wondered in the past if we realled needed those double
implementations...
Is there really a point in maintaining both ? I am pretty sure people only
use the default configuration per architecture.
  

>> On Thu, Dec 20, 2018 at 11:44 AM Gavin Hu <mailto:gavin.hu@arm.com> wrote:
>> remove the hard code #ifdef RTE_FORCE_INTRINSICS, move the implementation
>> to the arm specific file, x86 and POWER have their own implementations.

> No, x86 and ppc define their own implementation when _not_ having RTE_FORCE_INTRINSICS.
> David Marchand

Hi David,

Your reply is out of format, I re-format it to text based.

Yes, x86 and ppc define their own implementation, so this change is arm specific.
Only arm have RTE_FORCE_INTRINSICS, x86 and ppc don't define it in the config files.
  

>> > On Thu, Dec 20, 2018 at 1:47 PM David Marchand <mailto:david.marchand@redhat.com> wrote:
>> > On Thu, Dec 20, 2018 at 11:44 AM Gavin Hu <mailto:gavin.hu@arm.com> wrote:
>>>  remove the hard code #ifdef RTE_FORCE_INTRINSICS, move the implementation
>>>  to the arm specific file, x86 and POWER have their own implementations.

>> No, x86 and ppc define their own implementation when _not_ having RTE_FORCE_INTRINSICS.

> Actually, I wondered in the past if we realled needed those double implementations...
> Is there really a point in maintaining both ? I am pretty sure people only use the default configuration per architecture.
> David Marchand

X86 implementation is xchg instruction based, it is arch specific.
Ppc has its own code base, but in reality it is same as arm implementation, but in future, arm will change to its own instruction level based implementation.
  

On Thu, Dec 20, 2018 at 3:36 PM Gavin Hu (Arm Technology China) <
Gavin.Hu@arm.com> wrote:

> >> On Thu, Dec 20, 2018 at 11:44 AM Gavin Hu <mailto:gavin.hu@arm.com>
> wrote:
> >> remove the hard code #ifdef RTE_FORCE_INTRINSICS, move the
> implementation
> >> to the arm specific file, x86 and POWER have their own implementations.
>
> > No, x86 and ppc define their own implementation when _not_ having
> RTE_FORCE_INTRINSICS.
> > David Marchand
>
> Hi David,
>
> Your reply is out of format, I re-format it to text based.
>

I suppose this is an issue with your mail client.


> Yes, x86 and ppc define their own implementation, so this change is arm
> specific.
> Only arm have RTE_FORCE_INTRINSICS, x86 and ppc don't define it in the
> config files.
>

This change breaks the use of intrinsics in x86 case at least.

$ git reset --hard origin/master
HEAD is now at 476c847 malloc: add option --match-allocations
$ git am
~/Downloads/v1-4-5-spinlock-move-the-implementation-to-arm-specific-file.patch
Applying: spinlock: move the implementation to arm specific file

# default config
$ rm -rf master; make defconfig O=master && make -j4 O=master
[...]
Build complete [x86_64-native-linuxapp-gcc]

# then enable use of intrinsics
$ echo CONFIG_RTE_FORCE_INTRINSICS=y >> master/.config && make O=master
[...]
  CC eal.o
In file included from
/home/dmarchan/git/pub/dpdk/master/include/rte_spinlock.h:12:0,
                 from
/home/dmarchan/git/pub/dpdk/master/include/rte_malloc_heap.h:10,
                 from
/home/dmarchan/git/pub/dpdk/master/include/rte_eal_memconfig.h:12,
                 from
/home/dmarchan/git/pub/dpdk/lib/librte_eal/linuxapp/eal/eal.c:35:
/home/dmarchan/git/pub/dpdk/master/include/generic/rte_spinlock.h:58:1:
error: ‘rte_spinlock_lock’ used but never defined [-Werror]
 rte_spinlock_lock(rte_spinlock_t *sl);
 ^
/home/dmarchan/git/pub/dpdk/master/include/generic/rte_spinlock.h:67:1:
error: ‘rte_spinlock_unlock’ used but never defined [-Werror]
 rte_spinlock_unlock (rte_spinlock_t *sl);
 ^
/home/dmarchan/git/pub/dpdk/master/include/generic/rte_spinlock.h:78:1:
error: ‘rte_spinlock_trylock’ used but never defined [-Werror]
 rte_spinlock_trylock (rte_spinlock_t *sl);
 ^
cc1: all warnings being treated as errors
make[5]: *** [eal.o] Error 1
make[4]: *** [eal] Error 2
  

> This change breaks the use of intrinsics in x86 case at least.

This is true, if intrinsics is still an option for x86 even not is use by default.
I will fix this in v2.

Don't know why mail mail client has this issue, I worked with other mail threads correctly.
  

On Thu, Dec 20, 2018 at 4:58 PM Gavin Hu (Arm Technology China) <
Gavin.Hu@arm.com> wrote:

>
> > This change breaks the use of intrinsics in x86 case at least.
>
> This is true, if intrinsics is still an option for x86 even not is use by
> default.
> I will fix this in v2.
>

Please, also check ppc, same pb afaics.
Thanks.