eal/ppc: fix redefine bool type
Checks
Commit Message
The AltiVec header file breaks boolean type. [1] [2]
Currently the workaround was located only in mlx5 device.
Adding the trace module caused this issue to appear again, due to
order of includes, it keeps overriding the local fix.
This patch solves this issue by resetting the bool type, immediately
after it is being changed.
[1] https://mails.dpdk.org/archives/dev/2018-August/110281.html
[2]
In file included from
dpdk/ppc_64-power8-linux-gcc/include/rte_mempool_trace_fp.h:18:0,
from
dpdk/ppc_64-power8-linux-gcc/include/rte_mempool.h:54,
from
dpdk/drivers/common/mlx5/mlx5_common_mr.c:7:
dpdk/ppc_64-power8-linux-gcc/include/rte_trace_point.h: In
function '__rte_trace_point_fp_is_enabled':
dpdk/ppc_64-power8-linux-gcc/include/rte_trace_point.h:226:2:
error: incompatible types when returning type 'int' but '__vector __bool
int' was expected
return false;
^
In file included from
dpdk/ppc_64-power8-linux-gcc/include/rte_trace_point.h:281:0,
from
dpdk/ppc_64-power8-linux-gcc/include/rte_mempool_trace_fp.h:18,
from
dpdk/ppc_64-power8-linux-gcc/include/rte_mempool.h:54,
from
dpdk/drivers/common/mlx5/mlx5_common_mr.c:7:
dpdk/ppc_64-power8-linux-gcc/include/rte_mempool_trace_fp.h:
In function 'rte_mempool_trace_ops_dequeue_bulk':
dpdk/ppc_64-power8-linux-gcc/include/rte_trace_point_provider.h:104:6:
error: wrong type argument to unary exclamation mark
if (!__rte_trace_point_fp_is_enabled()) \
^
dpdk/ppc_64-power8-linux-gcc/include/rte_trace_point.h:49:2:
note: in expansion of macro '__rte_trace_point_emit_header_fp'
__rte_trace_point_emit_header_##_mode(&__##_tp); \
^
dpdk/ppc_64-power8-linux-gcc/include/rte_trace_point.h:99:2:
note: in expansion of macro '__RTE_TRACE_POINT'
__RTE_TRACE_POINT(fp, tp, args, __VA_ARGS__)
^
dpdk/ppc_64-power8-linux-gcc/include/rte_mempool_trace_fp.h:20:1:
note: in expansion of macro 'RTE_TRACE_POINT_FP'
RTE_TRACE_POINT_FP(
^
dpdk/ppc_64-power8-linux-gcc/include/rte_mempool_trace_fp.h:
In function 'rte_mempool_trace_ops_dequeue_contig_blocks':
dpdk/ppc_64-power8-linux-gcc/include/rte_trace_point_provider.h:104:6:
error: wrong type argument to unary exclamation mark
if (!__rte_trace_point_fp_is_enabled()) \
^
dpdk/ppc_64-power8-linux-gcc/include/rte_trace_point.h:49:2:
note: in expansion of macro '__rte_trace_point_emit_header_fp'
__rte_trace_point_emit_header_##_mode(&__##_tp); \
^
dpdk/ppc_64-power8-linux-gcc/include/rte_trace_point.h:99:2:
note: in expansion of macro '__RTE_TRACE_POINT'
__RTE_TRACE_POINT(fp, tp, args, __VA_ARGS__)
^
dpdk/ppc_64-power8-linux-gcc/include/rte_mempool_trace_fp.h:29:1:
note: in expansion of macro 'RTE_TRACE_POINT_FP'
RTE_TRACE_POINT_FP(
^
dpdk/ppc_64-power8-linux-gcc/include/rte_mempool_trace_fp.h:
In function 'rte_mempool_trace_ops_enqueue_bulk':
dpdk/ppc_64-power8-linux-gcc/include/rte_trace_point_provider.h:104:6:
error: wrong type argument to unary exclamation mark
if (!__rte_trace_point_fp_is_enabled()) \
Fixes: 725f5dd0bfb5 ("net/mlx5: fix build on PPC64")
Signed-off-by: Ori Kam <orika@mellanox.com>
---
drivers/common/mlx5/mlx5_common.h | 10 ----------
drivers/net/mlx5/mlx5_utils.h | 10 ----------
lib/librte_eal/ppc/include/meson.build | 1 +
lib/librte_eal/ppc/include/rte_altivec.h | 22 ++++++++++++++++++++++
lib/librte_eal/ppc/include/rte_memcpy.h | 3 +--
5 files changed, 24 insertions(+), 22 deletions(-)
create mode 100644 lib/librte_eal/ppc/include/rte_altivec.h
Comments
> Fixes: 725f5dd0bfb5 ("net/mlx5: fix build on PPC64")
>
> Signed-off-by: Ori Kam <orika@mellanox.com>
> ---
There are a couple of other uses that should be covered in the patch:
diff --git a/drivers/net/i40e/i40e_rxtx_vec_altivec.c
b/drivers/net/i40e/i40e_rxtx_vec_altivec.c
index 5fa92bf92..72bd410fc 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_altivec.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_altivec.c
@@ -13,7 +13,7 @@
#include "i40e_rxtx.h"
#include "i40e_rxtx_vec_common.h"
-#include <altivec.h>
+#include "rte_altivec.h"
#pragma GCC diagnostic ignored "-Wcast-qual"
diff --git a/drivers/net/virtio/virtio_rxtx_simple_altivec.c
b/drivers/net/virtio/virtio_rxtx_simple_altivec.c
index 47225f412..b2bdd05c8 100644
--- a/drivers/net/virtio/virtio_rxtx_simple_altivec.c
+++ b/drivers/net/virtio/virtio_rxtx_simple_altivec.c
@@ -9,7 +9,7 @@
#include <string.h>
#include <errno.h>
-#include <altivec.h>
+#include "rte_altivec.h"
#include <rte_byteorder.h>
#include <rte_branch_prediction.h>
Dave
On Tue, Apr 28, 2020 at 9:59 AM Ori Kam <orika@mellanox.com> wrote:
>
> The AltiVec header file breaks boolean type. [1] [2]
>
> Currently the workaround was located only in mlx5 device.
> Adding the trace module caused this issue to appear again, due to
> order of includes, it keeps overriding the local fix.
>
> This patch solves this issue by resetting the bool type, immediately
> after it is being changed.
With this patch applied, there are still a few remaining spots as
mentioned by David C.
I see rte_vect.h too.
$ git grep -w altivec.h
MAINTAINERS:F: examples/l3fwd/*altivec.h
drivers/net/i40e/i40e_rxtx_vec_altivec.c:#include <altivec.h>
drivers/net/mlx5/mlx5_rxtx_vec_altivec.h:#include <altivec.h>
drivers/net/virtio/virtio_rxtx_simple_altivec.c:#include <altivec.h>
lib/librte_eal/ppc/include/rte_altivec.h:/* To include altivec.h, GCC
version must be >= 4.8 */
lib/librte_eal/ppc/include/rte_altivec.h:#include <altivec.h>
lib/librte_eal/ppc/include/rte_vect.h:#include <altivec.h>
> diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h b/lib/librte_eal/ppc/include/rte_memcpy.h
> index 25311ba..d234e21 100644
> --- a/lib/librte_eal/ppc/include/rte_memcpy.h
> +++ b/lib/librte_eal/ppc/include/rte_memcpy.h
> @@ -8,13 +8,12 @@
>
> #include <stdint.h>
> #include <string.h>
> -/*To include altivec.h, GCC version must >= 4.8 */
> -#include <altivec.h>
Why move the inclusion under the __cplusplus check?
>
> #ifdef __cplusplus
> extern "C" {
> #endif
>
> +#include "rte_altivec.h"
> #include "generic/rte_memcpy.h"
>
> static inline void
> --
> 1.8.3.1
>
Thanks.
Hi David,
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, April 29, 2020 11:17 AM
> To: Ori Kam <orika@mellanox.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; Matan Azrad
> <matan@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>; Slava
> Ovsiienko <viacheslavo@mellanox.com>; David Christensen
> <drc@linux.vnet.ibm.com>; dev <dev@dpdk.org>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] eal/ppc: fix redefine bool type
>
> On Tue, Apr 28, 2020 at 9:59 AM Ori Kam <orika@mellanox.com> wrote:
> >
> > The AltiVec header file breaks boolean type. [1] [2]
> >
> > Currently the workaround was located only in mlx5 device.
> > Adding the trace module caused this issue to appear again, due to
> > order of includes, it keeps overriding the local fix.
> >
> > This patch solves this issue by resetting the bool type, immediately
> > after it is being changed.
>
>
> With this patch applied, there are still a few remaining spots as
> mentioned by David C.
> I see rte_vect.h too.
>
I will add the missing code.
> $ git grep -w altivec.h
> MAINTAINERS:F: examples/l3fwd/*altivec.h
> drivers/net/i40e/i40e_rxtx_vec_altivec.c:#include <altivec.h>
> drivers/net/mlx5/mlx5_rxtx_vec_altivec.h:#include <altivec.h>
> drivers/net/virtio/virtio_rxtx_simple_altivec.c:#include <altivec.h>
> lib/librte_eal/ppc/include/rte_altivec.h:/* To include altivec.h, GCC
> version must be >= 4.8 */
> lib/librte_eal/ppc/include/rte_altivec.h:#include <altivec.h>
> lib/librte_eal/ppc/include/rte_vect.h:#include <altivec.h>
>
>
> > diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h
> b/lib/librte_eal/ppc/include/rte_memcpy.h
> > index 25311ba..d234e21 100644
> > --- a/lib/librte_eal/ppc/include/rte_memcpy.h
> > +++ b/lib/librte_eal/ppc/include/rte_memcpy.h
> > @@ -8,13 +8,12 @@
> >
> > #include <stdint.h>
> > #include <string.h>
> > -/*To include altivec.h, GCC version must >= 4.8 */
> > -#include <altivec.h>
>
> Why move the inclusion under the __cplusplus check?
>
Just to make it in the same part as other rte includes.
>
> >
> > #ifdef __cplusplus
> > extern "C" {
> > #endif
> >
> > +#include "rte_altivec.h"
> > #include "generic/rte_memcpy.h"
> >
> > static inline void
> > --
> > 1.8.3.1
> >
>
> Thanks.
>
> --
> David Marchand
Hi David,
I will add your code.
Thanks,
Ori
> -----Original Message-----
> From: David Christensen <drc@linux.vnet.ibm.com>
> Sent: Tuesday, April 28, 2020 9:20 PM
> To: Ori Kam <orika@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; Matan Azrad <matan@mellanox.com>; Shahaf
> Shuler <shahafs@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; ferruh.yigit@intel.com
> Subject: Re: [PATCH] eal/ppc: fix redefine bool type
>
> > Fixes: 725f5dd0bfb5 ("net/mlx5: fix build on PPC64")
> >
> > Signed-off-by: Ori Kam <orika@mellanox.com>
> > ---
>
> There are a couple of other uses that should be covered in the patch:
>
> diff --git a/drivers/net/i40e/i40e_rxtx_vec_altivec.c
> b/drivers/net/i40e/i40e_rxtx_vec_altivec.c
> index 5fa92bf92..72bd410fc 100644
> --- a/drivers/net/i40e/i40e_rxtx_vec_altivec.c
> +++ b/drivers/net/i40e/i40e_rxtx_vec_altivec.c
> @@ -13,7 +13,7 @@
> #include "i40e_rxtx.h"
> #include "i40e_rxtx_vec_common.h"
>
> -#include <altivec.h>
> +#include "rte_altivec.h"
>
> #pragma GCC diagnostic ignored "-Wcast-qual"
>
>
> diff --git a/drivers/net/virtio/virtio_rxtx_simple_altivec.c
> b/drivers/net/virtio/virtio_rxtx_simple_altivec.c
> index 47225f412..b2bdd05c8 100644
> --- a/drivers/net/virtio/virtio_rxtx_simple_altivec.c
> +++ b/drivers/net/virtio/virtio_rxtx_simple_altivec.c
> @@ -9,7 +9,7 @@
> #include <string.h>
> #include <errno.h>
>
> -#include <altivec.h>
> +#include "rte_altivec.h"
>
> #include <rte_byteorder.h>
> #include <rte_branch_prediction.h>
>
> Dave
On Thu, Apr 30, 2020 at 10:53 AM Ori Kam <orika@mellanox.com> wrote:
> > > diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h
> > b/lib/librte_eal/ppc/include/rte_memcpy.h
> > > index 25311ba..d234e21 100644
> > > --- a/lib/librte_eal/ppc/include/rte_memcpy.h
> > > +++ b/lib/librte_eal/ppc/include/rte_memcpy.h
> > > @@ -8,13 +8,12 @@
> > >
> > > #include <stdint.h>
> > > #include <string.h>
> > > -/*To include altivec.h, GCC version must >= 4.8 */
> > > -#include <altivec.h>
> >
> > Why move the inclusion under the __cplusplus check?
> >
> Just to make it in the same part as other rte includes.
"Normal" rte includes are usually standalone and "#ifdef __cplusplus" safe.
The rte_altivec.h header you added does not need any "#ifdef
__cplusplus" protection, but it might later).
But otoh, "generic/" headers are special/internal headers and this is
why generic/rte_memcpy.h is under this check.
So if there is no reason on your side, please leave rte_altivec.h
inclusion at the same place as the previous altivec.h.
Thanks.
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, April 30, 2020 12:04 PM
> To: Ori Kam <orika@mellanox.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; Matan Azrad
> <matan@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>; Slava
> Ovsiienko <viacheslavo@mellanox.com>; David Christensen
> <drc@linux.vnet.ibm.com>; dev <dev@dpdk.org>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] eal/ppc: fix redefine bool type
>
> On Thu, Apr 30, 2020 at 10:53 AM Ori Kam <orika@mellanox.com> wrote:
> > > > diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h
> > > b/lib/librte_eal/ppc/include/rte_memcpy.h
> > > > index 25311ba..d234e21 100644
> > > > --- a/lib/librte_eal/ppc/include/rte_memcpy.h
> > > > +++ b/lib/librte_eal/ppc/include/rte_memcpy.h
> > > > @@ -8,13 +8,12 @@
> > > >
> > > > #include <stdint.h>
> > > > #include <string.h>
> > > > -/*To include altivec.h, GCC version must >= 4.8 */
> > > > -#include <altivec.h>
> > >
> > > Why move the inclusion under the __cplusplus check?
> > >
> > Just to make it in the same part as other rte includes.
>
> "Normal" rte includes are usually standalone and "#ifdef __cplusplus" safe.
> The rte_altivec.h header you added does not need any "#ifdef
> __cplusplus" protection, but it might later).
>
> But otoh, "generic/" headers are special/internal headers and this is
> why generic/rte_memcpy.h is under this check.
>
> So if there is no reason on your side, please leave rte_altivec.h
> inclusion at the same place as the previous altivec.h.
>
Sure, I will leave at the original place.
Best,
Ori
>
> Thanks.
>
> --
> David Marchand
@@ -17,16 +17,6 @@
#include "mlx5_prm.h"
-/*
- * Compilation workaround for PPC64 when AltiVec is fully enabled, e.g. std=c11.
- * Otherwise there would be a type conflict between stdbool and altivec.
- */
-#if defined(__PPC64__) && !defined(__APPLE_ALTIVEC__)
-#undef bool
-/* redefine as in stdbool.h */
-#define bool _Bool
-#endif
-
/* Bit-field manipulation. */
#define BITFIELD_DECLARE(bf, type, size) \
type bf[(((size_t)(size) / (sizeof(type) * CHAR_BIT)) + \
@@ -21,16 +21,6 @@
#include "mlx5_defs.h"
-/*
- * Compilation workaround for PPC64 when AltiVec is fully enabled, e.g. std=c11.
- * Otherwise there would be a type conflict between stdbool and altivec.
- */
-#if defined(__PPC64__) && !defined(__APPLE_ALTIVEC__)
-#undef bool
-/* redefine as in stdbool.h */
-#define bool _Bool
-#endif
-
/* Convert a bit number to the corresponding 64-bit mask */
#define MLX5_BITSHIFT(v) (UINT64_C(1) << (v))
@@ -4,6 +4,7 @@
includes += include_directories('.')
arch_headers = files(
+ 'rte_altivec.h',
'rte_atomic.h',
'rte_byteorder.h',
'rte_cpuflags.h',
new file mode 100644
@@ -0,0 +1,22 @@
+/*
+ * SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) Mellanox 2020.
+ */
+
+#ifndef _RTE_ALTIVEC_H_
+#define _RTE_ALTIVEC_H_
+
+/* To include altivec.h, GCC version must be >= 4.8 */
+#include <altivec.h>
+
+/*
+ * Compilation workaround for PPC64 when AltiVec is fully enabled, e.g. std=c11.
+ * Otherwise there would be a type conflict between stdbool and altivec.
+ */
+#if defined(__PPC64__) && !defined(__APPLE_ALTIVEC__)
+#undef bool
+/* redefine as in stdbool.h */
+#define bool _Bool
+#endif
+
+#endif /* _RTE_ALTIVEC_H_ */
@@ -8,13 +8,12 @@
#include <stdint.h>
#include <string.h>
-/*To include altivec.h, GCC version must >= 4.8 */
-#include <altivec.h>
#ifdef __cplusplus
extern "C" {
#endif
+#include "rte_altivec.h"
#include "generic/rte_memcpy.h"
static inline void