Re: [PATCH 0/3] Fix xmm_t to rte_xmm_t scalar conversion

["Stanisław Kardach" <kda@semihalf.com>]

On Tue, Jun 21, 2022 at 12:54 AM David Christensen
<drc@linux.vnet.ibm.com> wrote:
>
> > On Thu, Jun 9, 2022 at 2:17 PM Stanislaw Kardach <kda@semihalf.com> wrote:
> >>
> >> As David noticed in [1] there is an issue with C++ compilation of the
> >> rte_vect.h header in RISC-V. Upon closer inspection, the problem appears on
> >> all architectures due to the type conversion rules in C++.
> >> More precisely a union type rte_xmm_t requires a conversion constructor
> >> from xmm_t type.
> >> The most obvious fix is to use a structure initializer for such copies
> >> (since rte_xmm_t union contains xmm_t anyway). The generated assembly
> >> at -O2 is exactly the same, so there's no real impact.
> >>
> >> The bigger question is whether accessing bits of the architecture specific
> >> xmm_t type in an array fashion is always correct? All current architectures
> >> define rte_xmm_t in the same manner implying that.
> >
> > Copying other arch maintainers.
>
> My read of the Altivec vector layout for LE systems says the existing
> union operator rte_xmm_t is correct, though my C++ experience is
> limited.  How can I generate an error with C++ to expose this issue?
>
> Dave
To replicate this issue:
1. Apply the patch below. In essence it forces the use of scalar lpm
and changes C++ compiler to g++ so that meson properly detects it.
Otherwise C++ checks won't be generated.
2. Configure build with: meson build-ppc64le --werror --cross-file
config/ppc/ppc64le-power8-linux-gcc-ubuntu -Dcheck_includes=true
3. Build with: ninja -C build-ppc64le buildtools/chkincs/chkincs-cpp

Note that the build target only gets generated if C++ compiler is
properly discovered by meson. To be honest I'm not sure why
powerpc64le-linux-gnu-cpp doesn't get properly picked up by meson.
Also of interesting note the similar substitution should be made for
arm64 but it seems like a separate thread.

Patch for issue replication:

diff --git a/config/arm/arm64_armv8_linux_gcc b/config/arm/arm64_armv8_linux_gcc
index 5391d35389..5c32f6b9ca 100644
--- a/config/arm/arm64_armv8_linux_gcc
+++ b/config/arm/arm64_armv8_linux_gcc
@@ -1,6 +1,6 @@
 [binaries]
 c = 'aarch64-linux-gnu-gcc'
-cpp = 'aarch64-linux-gnu-cpp'
+cpp = 'aarch64-linux-gnu-g++'
 ar = 'aarch64-linux-gnu-gcc-ar'
 strip = 'aarch64-linux-gnu-strip'
 pkgconfig = 'aarch64-linux-gnu-pkg-config'
diff --git a/config/ppc/ppc64le-power8-linux-gcc-ubuntu
b/config/ppc/ppc64le-power8-linux-gcc-ubuntu
index 803c612cbc..2e6b13a406 100644
--- a/config/ppc/ppc64le-power8-linux-gcc-ubuntu
+++ b/config/ppc/ppc64le-power8-linux-gcc-ubuntu
@@ -1,6 +1,6 @@
 [binaries]
 c = 'powerpc64le-linux-gnu-gcc'
-cpp = 'powerpc64le-linux-gnu-cpp'
+cpp = 'powerpc64le-linux-gnu-g++'
 ar = 'powerpc64le-linux-gnu-ar'
 strip = 'powerpc64le-linux-gnu-strip'

diff --git a/lib/lpm/rte_lpm.h b/lib/lpm/rte_lpm.h
index 4f38864fde..fd1362a027 100644
--- a/lib/lpm/rte_lpm.h
+++ b/lib/lpm/rte_lpm.h
@@ -397,19 +397,7 @@ static inline void
 rte_lpm_lookupx4(const struct rte_lpm *lpm, xmm_t ip, uint32_t hop[4],
        uint32_t defv);

-#if defined(RTE_ARCH_ARM)
-#ifdef RTE_HAS_SVE_ACLE
-#include "rte_lpm_sve.h"
-#else
-#include "rte_lpm_neon.h"
-#endif
-#elif defined(RTE_ARCH_PPC_64)
-#include "rte_lpm_altivec.h"
-#elif defined(RTE_ARCH_X86)
-#include "rte_lpm_sse.h"
-#else
 #include "rte_lpm_scalar.h"
-#endif

 #ifdef __cplusplus
 }


-- 
Best Regards,
Stanisław Kardach