[dpdk-dev] [PATCH] arm: detect NEON by RTE_MACHINE_CPUFLAG_NEON flag only

[dpdk-dev] [PATCH] arm: detect NEON by RTE_MACHINE_CPUFLAG_NEON flag only

[Jan Viktorin]

The RTE_MACHINE_CPUFLAG_NEON was only a result of the gcc testing.
However, the target CPU may not support NEON or the user can disable to
use it (as it does not always improve the performance).

The RTE_MACHINE_CPUFLAG_NEON detection is now based on both, the
__ARM_NEON_FP feature from gcc and CONFIG_RTE_ARCH_ARM_NEON from
the .config. The memcpy implemention is driven by
RTE_MACHINE_CPUFLAG_NEON, so the reason to disable neon is hidden for
the actual code.

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
---

I can also include this one:

examples/l3fwd/l3fwd_em.c:253:#elif defined(__ARM_NEON)

---
 lib/librte_eal/common/include/arch/arm/rte_memcpy_32.h | 4 ++--
 mk/machine/armv7a/rte.vars.mk                          | 2 +-
 mk/rte.cpuflags.mk                                     | 2 ++
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/include/arch/arm/rte_memcpy_32.h
b/lib/librte_eal/common/include/arch/arm/rte_memcpy_32.h index
df47c0d..ad8bc65 100644 ---
a/lib/librte_eal/common/include/arch/arm/rte_memcpy_32.h +++
b/lib/librte_eal/common/include/arch/arm/rte_memcpy_32.h @@ -42,7 +42,7
@@ extern "C" { 
 #include "generic/rte_memcpy.h"
 
-#ifdef __ARM_NEON_FP
+#ifdef RTE_MACHINE_CPUFLAG_NEON
 
 /* ARM NEON Intrinsics are used to copy data */
 #include <arm_neon.h>
@@ -325,7 +325,7 @@ rte_memcpy_func(void *dst, const void *src, size_t
n) return memcpy(dst, src, n);
 }
 
-#endif /* __ARM_NEON_FP */
+#endif /* RTE_MACHINE_CPUFLAG_NEON */
 
 #ifdef __cplusplus
 }
diff --git a/mk/machine/armv7a/rte.vars.mk
b/mk/machine/armv7a/rte.vars.mk index 48d3979..7a167c1 100644
--- a/mk/machine/armv7a/rte.vars.mk
+++ b/mk/machine/armv7a/rte.vars.mk
@@ -62,6 +62,6 @@ ifdef CONFIG_RTE_ARCH_ARM_TUNE
 MACHINE_CFLAGS += -mtune=$(CONFIG_RTE_ARCH_ARM_TUNE)
 endif
 
-ifeq ($(CONFIG_RTE_ARCH_ARM_NEON),y)
+ifdef $(RTE_MACHINE_CPUFLAG_NEON)
 MACHINE_CFLAGS += -mfpu=neon
 endif
diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk
index 19a3e7e..1947511 100644
--- a/mk/rte.cpuflags.mk
+++ b/mk/rte.cpuflags.mk
@@ -111,9 +111,11 @@ CPUFLAGS += VSX
 endif
 
 # ARM flags
+ifeq ($(CONFIG_RTE_ARCH_ARM_NEON),y)
 ifneq ($(filter $(AUTO_CPUFLAGS),__ARM_NEON_FP),)
 CPUFLAGS += NEON
 endif
+endif
 
 ifneq ($(filter $(AUTO_CPUFLAGS),__ARM_FEATURE_CRC32),)
 CPUFLAGS += CRC32
-- 
2.7.0

Re: [dpdk-dev] [PATCH] arm: detect NEON by RTE_MACHINE_CPUFLAG_NEON flag only

[Thomas Monjalon]

2016-03-18 13:00, Jan Viktorin:
> The RTE_MACHINE_CPUFLAG_NEON was only a result of the gcc testing.
> However, the target CPU may not support NEON or the user can disable to
> use it (as it does not always improve the performance).
> 
> The RTE_MACHINE_CPUFLAG_NEON detection is now based on both, the
> __ARM_NEON_FP feature from gcc and CONFIG_RTE_ARCH_ARM_NEON from
> the .config. The memcpy implemention is driven by
> RTE_MACHINE_CPUFLAG_NEON, so the reason to disable neon is hidden for
> the actual code.
> 
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> ---
> 
> I can also include this one:
> 
> examples/l3fwd/l3fwd_em.c:253:#elif defined(__ARM_NEON)

Yes please.
I will set my patch as superseded.

Re: [dpdk-dev] [PATCH] arm: detect NEON by RTE_MACHINE_CPUFLAG_NEON flag only

[Jan Viktorin]

On Fri, 18 Mar 2016 14:49:57 +0100
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2016-03-18 13:00, Jan Viktorin:
> > The RTE_MACHINE_CPUFLAG_NEON was only a result of the gcc testing.
> > However, the target CPU may not support NEON or the user can disable to
> > use it (as it does not always improve the performance).
> > 
> > The RTE_MACHINE_CPUFLAG_NEON detection is now based on both, the
> > __ARM_NEON_FP feature from gcc and CONFIG_RTE_ARCH_ARM_NEON from
> > the .config. The memcpy implemention is driven by
> > RTE_MACHINE_CPUFLAG_NEON, so the reason to disable neon is hidden for
> > the actual code.
> > 
> > Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> > ---
> > 
> > I can also include this one:
> > 
> > examples/l3fwd/l3fwd_em.c:253:#elif defined(__ARM_NEON)  
> 
> Yes please.
> I will set my patch as superseded.
> 

OK, I will send v2.

By the way, for Intel-related code, it is also common to check eg.
__SSE2__ instead of the RTE_MACHINE_CPUFLAG_SSE2. That's probably a
source of confusion for new code, newcomers and adding new platforms.

As for me, I've had known about the CPUFLAGs... But I could hardly see
those in the DPDK code. It looks like the features are detected by
unused... And IMHO this is the reason why we are confused here.

Regards
Jan