rte_trace: fix build on PPC64

Message ID 20200427215849.116064-1-thinhtr@linux.vnet.ibm.com (mailing list archive)
State Rejected, archived
Delegated to: David Marchand
Headers
Series rte_trace: fix build on PPC64 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/travis-robot success Travis build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Thinh Tran April 27, 2020, 9:58 p.m. UTC
  The AltiVec header file breaks boolean type:

In file included from ../lib/librte_mempool/rte_mempool_trace_fp.h:18:0,
                 from ../lib/librte_mempool/rte_mempool.h:54,
                 from ../lib/librte_mbuf/rte_mbuf.h:38,
                 from ../lib/librte_net/rte_ether.h:23,
                 from ../drivers/common/mlx5/mlx5_nl.h:10,
                 from ../drivers/common/mlx5/mlx5_nl.c:23:
../lib/librte_eal/include/rte_trace_point.h: In function
‘__rte_trace_point_fp_is_enabled’:
../lib/librte_eal/include/rte_trace_point.h:226:9: error: incompatible
types when returning type ‘int’ but ‘__vector __bool int {aka
__vector(4) __bool int}’ was expected
  return false;

This is the same as
 https://git.dpdk.org/dpdk/commit/?id=725f5dd

and yet, there is no better solution for it

Signed-off-by: Thinh Tran <thinhtr@linux.vnet.ibm.com>
---
 lib/librte_eal/include/rte_trace_point.h | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

Jerin Jacob April 28, 2020, 7:57 a.m. UTC | #1
On Tue, Apr 28, 2020 at 3:29 AM Thinh Tran <thinhtr@linux.vnet.ibm.com> wrote:
>
> The AltiVec header file breaks boolean type:
>
> In file included from ../lib/librte_mempool/rte_mempool_trace_fp.h:18:0,
>                  from ../lib/librte_mempool/rte_mempool.h:54,
>                  from ../lib/librte_mbuf/rte_mbuf.h:38,
>                  from ../lib/librte_net/rte_ether.h:23,
>                  from ../drivers/common/mlx5/mlx5_nl.h:10,
>                  from ../drivers/common/mlx5/mlx5_nl.c:23:
> ../lib/librte_eal/include/rte_trace_point.h: In function
> ‘__rte_trace_point_fp_is_enabled’:
> ../lib/librte_eal/include/rte_trace_point.h:226:9: error: incompatible
> types when returning type ‘int’ but ‘__vector __bool int {aka
> __vector(4) __bool int}’ was expected
>   return false;
>
> This is the same as
>  https://git.dpdk.org/dpdk/commit/?id=725f5dd
>
> and yet, there is no better solution for it
>
> Signed-off-by: Thinh Tran <thinhtr@linux.vnet.ibm.com>
> ---
>  lib/librte_eal/include/rte_trace_point.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/lib/librte_eal/include/rte_trace_point.h b/lib/librte_eal/include/rte_trace_point.h
> index 4d956ec16..2ede9e3ba 100644
> --- a/lib/librte_eal/include/rte_trace_point.h
> +++ b/lib/librte_eal/include/rte_trace_point.h
> @@ -26,6 +26,12 @@ extern "C" {
>  #include <rte_common.h>
>  #include <rte_compat.h>
>
> +#if defined(__PPC64__) && !defined(__APPLE_ALTIVEC__)
> +#undef bool
> +/* redefine as in stdbool.h */
> +#define bool _Bool
> +#endif

NACK.

Please move the fix to rte_common.h or similar as it not specific to trace.
if you do so, the following hack also not need.
https://git.dpdk.org/dpdk/commit/?id=725f5dd

> +
>  /** The tracepoint object. */
>  typedef uint64_t rte_trace_point_t;
>
> --
> 2.17.1
>
  
David Marchand April 28, 2020, 8:52 a.m. UTC | #2
On Tue, Apr 28, 2020 at 9:58 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Tue, Apr 28, 2020 at 3:29 AM Thinh Tran <thinhtr@linux.vnet.ibm.com> wrote:
> >
> > The AltiVec header file breaks boolean type:
> >
> > In file included from ../lib/librte_mempool/rte_mempool_trace_fp.h:18:0,
> >                  from ../lib/librte_mempool/rte_mempool.h:54,
> >                  from ../lib/librte_mbuf/rte_mbuf.h:38,
> >                  from ../lib/librte_net/rte_ether.h:23,
> >                  from ../drivers/common/mlx5/mlx5_nl.h:10,
> >                  from ../drivers/common/mlx5/mlx5_nl.c:23:
> > ../lib/librte_eal/include/rte_trace_point.h: In function
> > ‘__rte_trace_point_fp_is_enabled’:
> > ../lib/librte_eal/include/rte_trace_point.h:226:9: error: incompatible
> > types when returning type ‘int’ but ‘__vector __bool int {aka
> > __vector(4) __bool int}’ was expected
> >   return false;
> >
> > This is the same as
> >  https://git.dpdk.org/dpdk/commit/?id=725f5dd
> >
> > and yet, there is no better solution for it
> >
> > Signed-off-by: Thinh Tran <thinhtr@linux.vnet.ibm.com>
> > ---
> >  lib/librte_eal/include/rte_trace_point.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/lib/librte_eal/include/rte_trace_point.h b/lib/librte_eal/include/rte_trace_point.h
> > index 4d956ec16..2ede9e3ba 100644
> > --- a/lib/librte_eal/include/rte_trace_point.h
> > +++ b/lib/librte_eal/include/rte_trace_point.h
> > @@ -26,6 +26,12 @@ extern "C" {
> >  #include <rte_common.h>
> >  #include <rte_compat.h>
> >
> > +#if defined(__PPC64__) && !defined(__APPLE_ALTIVEC__)
> > +#undef bool
> > +/* redefine as in stdbool.h */
> > +#define bool _Bool
> > +#endif
>
> NACK.
>
> Please move the fix to rte_common.h or similar as it not specific to trace.
> if you do so, the following hack also not need.
> https://git.dpdk.org/dpdk/commit/?id=725f5dd

+1
  
Thinh Tran April 28, 2020, 3:01 p.m. UTC | #3
Agreed, this is not rte_trace issue. Moving to rte_common.h does not 
work neither, because altivec.h is included after and redefine bool
This conflict happens on PPC64, I'll create a different patch.

Thanks,
Thinh Tran

On 4/28/2020 2:57 AM, Jerin Jacob wrote:
> On Tue, Apr 28, 2020 at 3:29 AM Thinh Tran <thinhtr@linux.vnet.ibm.com> wrote:
>>
>> The AltiVec header file breaks boolean type:
>>
>> In file included from ../lib/librte_mempool/rte_mempool_trace_fp.h:18:0,
>>                   from ../lib/librte_mempool/rte_mempool.h:54,
>>                   from ../lib/librte_mbuf/rte_mbuf.h:38,
>>                   from ../lib/librte_net/rte_ether.h:23,
>>                   from ../drivers/common/mlx5/mlx5_nl.h:10,
>>                   from ../drivers/common/mlx5/mlx5_nl.c:23:
>> ../lib/librte_eal/include/rte_trace_point.h: In function
>> ‘__rte_trace_point_fp_is_enabled’:
>> ../lib/librte_eal/include/rte_trace_point.h:226:9: error: incompatible
>> types when returning type ‘int’ but ‘__vector __bool int {aka
>> __vector(4) __bool int}’ was expected
>>    return false;
>>
>> This is the same as
>>   https://git.dpdk.org/dpdk/commit/?id=725f5dd
>>
>> and yet, there is no better solution for it
>>
>> Signed-off-by: Thinh Tran <thinhtr@linux.vnet.ibm.com>
>> ---
>>   lib/librte_eal/include/rte_trace_point.h | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/lib/librte_eal/include/rte_trace_point.h b/lib/librte_eal/include/rte_trace_point.h
>> index 4d956ec16..2ede9e3ba 100644
>> --- a/lib/librte_eal/include/rte_trace_point.h
>> +++ b/lib/librte_eal/include/rte_trace_point.h
>> @@ -26,6 +26,12 @@ extern "C" {
>>   #include <rte_common.h>
>>   #include <rte_compat.h>
>>
>> +#if defined(__PPC64__) && !defined(__APPLE_ALTIVEC__)
>> +#undef bool
>> +/* redefine as in stdbool.h */
>> +#define bool _Bool
>> +#endif
> 
> NACK.
> 
> Please move the fix to rte_common.h or similar as it not specific to trace.
> if you do so, the following hack also not need.
> https://git.dpdk.org/dpdk/commit/?id=725f5dd
> 
>> +
>>   /** The tracepoint object. */
>>   typedef uint64_t rte_trace_point_t;
>>
>> --
>> 2.17.1
>>
  
David Marchand April 28, 2020, 3:04 p.m. UTC | #4
On Tue, Apr 28, 2020 at 5:02 PM Thinh Tran <thinhtr@linux.vnet.ibm.com> wrote:
>
> Agreed, this is not rte_trace issue. Moving to rte_common.h does not
> work neither, because altivec.h is included after and redefine bool
> This conflict happens on PPC64, I'll create a different patch.

Please look at Ori patch.
Thanks.

http://patchwork.dpdk.org/patch/69426/
  
Thinh Tran April 28, 2020, 3:26 p.m. UTC | #5
I looked at it, but I have a simpler patch below that can resolve the 
conflict, including after removing previous patches in mlx4/mlx5

Thanks,
Thinh Tran

  diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h 
b/lib/librte_eal/ppc/include/rte_memcpy.h
index 25311ba1d..abeede231 100644
--- a/lib/librte_eal/ppc/include/rte_memcpy.h
+++ b/lib/librte_eal/ppc/include/rte_memcpy.h
@@ -11,6 +11,10 @@
  /*To include altivec.h, GCC version must  >= 4.8 */
  #include <altivec.h>

+/* redefine as in stdbool.h */
+#undef bool
+#define bool _Bool
+
  #ifdef __cplusplus
  extern "C" {
  #endif

On 4/28/2020 10:04 AM, David Marchand wrote:
> On Tue, Apr 28, 2020 at 5:02 PM Thinh Tran <thinhtr@linux.vnet.ibm.com> wrote:
>>
>> Agreed, this is not rte_trace issue. Moving to rte_common.h does not
>> work neither, because altivec.h is included after and redefine bool
>> This conflict happens on PPC64, I'll create a different patch.
> 
> Please look at Ori patch.
> Thanks.
> 
> http://patchwork.dpdk.org/patch/69426/
>
  

Patch

diff --git a/lib/librte_eal/include/rte_trace_point.h b/lib/librte_eal/include/rte_trace_point.h
index 4d956ec16..2ede9e3ba 100644
--- a/lib/librte_eal/include/rte_trace_point.h
+++ b/lib/librte_eal/include/rte_trace_point.h
@@ -26,6 +26,12 @@  extern "C" {
 #include <rte_common.h>
 #include <rte_compat.h>
 
+#if defined(__PPC64__) && !defined(__APPLE_ALTIVEC__)
+#undef bool
+/* redefine as in stdbool.h */
+#define bool _Bool
+#endif
+
 /** The tracepoint object. */
 typedef uint64_t rte_trace_point_t;