[dpdk-dev] [PATCH] eal/ppc: fix redefine bool type

[dpdk-dev] [PATCH] eal/ppc: fix redefine bool type

[Ori Kam]

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

diff --git a/drivers/common/mlx5/mlx5_common.h b/drivers/common/mlx5/mlx5_common.h
index 16de1b3..c2d688a 100644
--- a/drivers/common/mlx5/mlx5_common.h
+++ b/drivers/common/mlx5/mlx5_common.h
@@ -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)) + \
diff --git a/drivers/net/mlx5/mlx5_utils.h b/drivers/net/mlx5/mlx5_utils.h
index d81ace3..0e016f8 100644
--- a/drivers/net/mlx5/mlx5_utils.h
+++ b/drivers/net/mlx5/mlx5_utils.h
@@ -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))
 
diff --git a/lib/librte_eal/ppc/include/meson.build b/lib/librte_eal/ppc/include/meson.build
index 3a91c98..3ee38f6 100644
--- a/lib/librte_eal/ppc/include/meson.build
+++ b/lib/librte_eal/ppc/include/meson.build
@@ -4,6 +4,7 @@
 includes += include_directories('.')
 
 arch_headers = files(
+	'rte_altivec.h',
 	'rte_atomic.h',
 	'rte_byteorder.h',
 	'rte_cpuflags.h',
diff --git a/lib/librte_eal/ppc/include/rte_altivec.h b/lib/librte_eal/ppc/include/rte_altivec.h
new file mode 100644
index 0000000..1551a94
--- /dev/null
+++ b/lib/librte_eal/ppc/include/rte_altivec.h
@@ -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_ */
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>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+#include "rte_altivec.h"
 #include "generic/rte_memcpy.h"
 
 static inline void
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] eal/ppc: fix redefine bool type

[David Christensen]

> 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

Re: [dpdk-dev] [PATCH] eal/ppc: fix redefine bool type

[Ori Kam]

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

Re: [dpdk-dev] [PATCH] eal/ppc: fix redefine bool type

[David Marchand]

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.

-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal/ppc: fix redefine bool type

[Ori Kam]

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

Re: [dpdk-dev] [PATCH] eal/ppc: fix redefine bool type

[David Marchand]

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.

-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal/ppc: fix redefine bool type

[Ori Kam]

> -----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

[dpdk-dev] [PATCH v2] eal/ppc: fix redefine bool type

[Ori Kam]

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>
Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
---
v2:
 * Replace alitvec include in all files.
 * Address ML comments. 
---
 drivers/common/mlx5/mlx5_common.h               | 10 ----------
 drivers/net/i40e/i40e_rxtx_vec_altivec.c        |  2 +-
 drivers/net/mlx5/mlx5_rxtx_vec_altivec.h        |  2 +-
 drivers/net/mlx5/mlx5_utils.h                   | 10 ----------
 drivers/net/virtio/virtio_rxtx_simple_altivec.c |  2 +-
 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 +--
 lib/librte_eal/ppc/include/rte_vect.h           |  2 +-
 9 files changed, 28 insertions(+), 26 deletions(-)
 create mode 100644 lib/librte_eal/ppc/include/rte_altivec.h

diff --git a/drivers/common/mlx5/mlx5_common.h b/drivers/common/mlx5/mlx5_common.h
index 16de1b3..c2d688a 100644
--- a/drivers/common/mlx5/mlx5_common.h
+++ b/drivers/common/mlx5/mlx5_common.h
@@ -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)) + \
diff --git a/drivers/net/i40e/i40e_rxtx_vec_altivec.c b/drivers/net/i40e/i40e_rxtx_vec_altivec.c
index 5fa92bf..72bd410 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/mlx5/mlx5_rxtx_vec_altivec.h b/drivers/net/mlx5/mlx5_rxtx_vec_altivec.h
index 45ff8e6..4559828 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec_altivec.h
+++ b/drivers/net/mlx5/mlx5_rxtx_vec_altivec.h
@@ -10,7 +10,7 @@
 #include <string.h>
 #include <stdlib.h>
 
-#include <altivec.h>
+#include "rte_altivec.h"
 
 #include <rte_mbuf.h>
 #include <rte_mempool.h>
diff --git a/drivers/net/mlx5/mlx5_utils.h b/drivers/net/mlx5/mlx5_utils.h
index d81ace3..0e016f8 100644
--- a/drivers/net/mlx5/mlx5_utils.h
+++ b/drivers/net/mlx5/mlx5_utils.h
@@ -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))
 
diff --git a/drivers/net/virtio/virtio_rxtx_simple_altivec.c b/drivers/net/virtio/virtio_rxtx_simple_altivec.c
index 47225f4..b2bdd05 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>
diff --git a/lib/librte_eal/ppc/include/meson.build b/lib/librte_eal/ppc/include/meson.build
index 3a91c98..3ee38f6 100644
--- a/lib/librte_eal/ppc/include/meson.build
+++ b/lib/librte_eal/ppc/include/meson.build
@@ -4,6 +4,7 @@
 includes += include_directories('.')
 
 arch_headers = files(
+	'rte_altivec.h',
 	'rte_atomic.h',
 	'rte_byteorder.h',
 	'rte_cpuflags.h',
diff --git a/lib/librte_eal/ppc/include/rte_altivec.h b/lib/librte_eal/ppc/include/rte_altivec.h
new file mode 100644
index 0000000..1551a94
--- /dev/null
+++ b/lib/librte_eal/ppc/include/rte_altivec.h
@@ -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_ */
diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h b/lib/librte_eal/ppc/include/rte_memcpy.h
index 25311ba..a49a516 100644
--- a/lib/librte_eal/ppc/include/rte_memcpy.h
+++ b/lib/librte_eal/ppc/include/rte_memcpy.h
@@ -8,8 +8,7 @@
 
 #include <stdint.h>
 #include <string.h>
-/*To include altivec.h, GCC version must  >= 4.8 */
-#include <altivec.h>
+#include "rte_altivec.h"
 
 #ifdef __cplusplus
 extern "C" {
diff --git a/lib/librte_eal/ppc/include/rte_vect.h b/lib/librte_eal/ppc/include/rte_vect.h
index 1d6c29b..b0545c8 100644
--- a/lib/librte_eal/ppc/include/rte_vect.h
+++ b/lib/librte_eal/ppc/include/rte_vect.h
@@ -6,7 +6,7 @@
 #ifndef _RTE_VECT_PPC_64_H_
 #define _RTE_VECT_PPC_64_H_
 
-#include <altivec.h>
+#include "rte_altivec.h"
 
 #include "generic/rte_vect.h"
 #include "rte_common.h"
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2] eal/ppc: fix redefine bool type

[David Christensen]

On 4/30/20 7:22 AM, Ori Kam 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.
...
> Fixes: 725f5dd0bfb5 ("net/mlx5: fix build on PPC64")
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
> ---
> v2:
>   * Replace alitvec include in all files.
>   * Address ML comments.
> ---
Tested-by: David Christensen <drc@linux.vnet.ibm.com>

Re: [dpdk-dev] [PATCH v2] eal/ppc: fix redefine bool type

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: David Christensen <drc@linux.vnet.ibm.com>
> Sent: Thursday, April 30, 2020 10:41 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>; Beilei Xing <beilei.xing@intel.com>; Qi Zhang
> <qi.z.zhang@intel.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Zhihong Wang
> <zhihong.wang@intel.com>; Xiaolong Ye <xiaolong.ye@intel.com>
> Cc: Raslan Darawsheh <rasland@mellanox.com>;
> david.marchand@redhat.com; dev@dpdk.org; ferruh.yigit@intel.com
> Subject: Re: [PATCH v2] eal/ppc: fix redefine bool type
> 
> 
> 
> On 4/30/20 7:22 AM, Ori Kam 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.
> ...
> > Fixes: 725f5dd0bfb5 ("net/mlx5: fix build on PPC64")
> >
> > Signed-off-by: Ori Kam <orika@mellanox.com>
> > Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
> > ---
> > v2:
> >   * Replace alitvec include in all files.
> >   * Address ML comments.
> > ---
> Tested-by: David Christensen <drc@linux.vnet.ibm.com>
Tested-by: Raslan Darawsheh <rasland@mellanox.com>

Kindest regards,
Raslan Darawsheh

Re: [dpdk-dev] [PATCH v2] eal/ppc: fix redefine bool type

[Matan Azrad]

From: Raslan Darawsheh
> Hi,
> 
> > -----Original Message-----
> > From: David Christensen <drc@linux.vnet.ibm.com>
> > Sent: Thursday, April 30, 2020 10:41 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>;
> > Beilei Xing <beilei.xing@intel.com>; Qi Zhang <qi.z.zhang@intel.com>;
> > Maxime Coquelin <maxime.coquelin@redhat.com>; Zhihong Wang
> > <zhihong.wang@intel.com>; Xiaolong Ye <xiaolong.ye@intel.com>
> > Cc: Raslan Darawsheh <rasland@mellanox.com>;
> > david.marchand@redhat.com; dev@dpdk.org; ferruh.yigit@intel.com
> > Subject: Re: [PATCH v2] eal/ppc: fix redefine bool type
> >
> >
> >
> > On 4/30/20 7:22 AM, Ori Kam 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.
> > ...
> > > Fixes: 725f5dd0bfb5 ("net/mlx5: fix build on PPC64")
> > >
> > > Signed-off-by: Ori Kam <orika@mellanox.com>
> > > Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
> > > ---
> > > v2:
> > >   * Replace alitvec include in all files.
> > >   * Address ML comments.
> > > ---
> > Tested-by: David Christensen <drc@linux.vnet.ibm.com>
> Tested-by: Raslan Darawsheh <rasland@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
> 
> Kindest regards,
> Raslan Darawsheh

Re: [dpdk-dev] [PATCH v2] eal/ppc: fix redefine bool type

[David Marchand]

On Thu, Apr 30, 2020 at 4:22 PM 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.
>
> [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>
> Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>

Tested-by: David Christensen <drc@linux.vnet.ibm.com>
Tested-by: Raslan Darawsheh <rasland@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>

Applied, thanks.


-- 
David Marchand