diff mbox series

eal/ppc: undefine AltiVec keyword vector

Message ID 20220525095307.675312-1-thomas@monjalon.net (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series eal/ppc: undefine AltiVec keyword vector |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Thomas Monjalon May 25, 2022, 9:53 a.m. UTC 
  The AltiVec header file is defining "vector", except in C++ build.
The keyword "vector" may conflict easily.
As a rule, it is better to use the alternative keyword "__vector".

The DPDK header file rte_altivec.h takes care of undefining "vector",
so the applications and dependencies are free to define the name "vector".

This is a compatibility breakage for applications which were using
the keyword "vector" for its AltiVec meaning.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 doc/guides/rel_notes/release_22_07.rst | 5 +++++
 lib/eal/ppc/include/rte_altivec.h      | 7 +++++++
 2 files changed, 12 insertions(+)

Comments

Tyler Retzlaff May 25, 2022, 10:01 a.m. UTC | #1 
On Wed, May 25, 2022 at 11:53:07AM +0200, Thomas Monjalon wrote:
> The AltiVec header file is defining "vector", except in C++ build.
> The keyword "vector" may conflict easily.
> As a rule, it is better to use the alternative keyword "__vector".
> 
> The DPDK header file rte_altivec.h takes care of undefining "vector",
> so the applications and dependencies are free to define the name "vector".
> 
> This is a compatibility breakage for applications which were using
> the keyword "vector" for its AltiVec meaning.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
Ali Alnubani May 25, 2022, 11:08 a.m. UTC | #2 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, May 25, 2022 12:53 PM
> To: dev@dpdk.org
> Cc: mdr@ashroe.eu; david.marchand@redhat.com; techboard@dpdk.org;
> David Christensen <drc@linux.vnet.ibm.com>
> Subject: [PATCH] eal/ppc: undefine AltiVec keyword vector
> 
> The AltiVec header file is defining "vector", except in C++ build.
> The keyword "vector" may conflict easily.
> As a rule, it is better to use the alternative keyword "__vector".
> 
> The DPDK header file rte_altivec.h takes care of undefining "vector",
> so the applications and dependencies are free to define the name "vector".
> 
> This is a compatibility breakage for applications which were using
> the keyword "vector" for its AltiVec meaning.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---

Build passes with latest rdma-core master (36395896) on Ubuntu 20.04.4.

Tested-by: Ali Alnubani <alialnu@nvidia.com>

Thanks,
Ali
Ali Alnubani May 25, 2022, 11:10 a.m. UTC | #3 
> -----Original Message-----
> From: Ali Alnubani
> Sent: Wednesday, May 25, 2022 2:09 PM
> To: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org
> Cc: mdr@ashroe.eu; david.marchand@redhat.com; techboard@dpdk.org;
> David Christensen <drc@linux.vnet.ibm.com>
> Subject: RE: [PATCH] eal/ppc: undefine AltiVec keyword vector
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Wednesday, May 25, 2022 12:53 PM
> > To: dev@dpdk.org
> > Cc: mdr@ashroe.eu; david.marchand@redhat.com; techboard@dpdk.org;
> > David Christensen <drc@linux.vnet.ibm.com>
> > Subject: [PATCH] eal/ppc: undefine AltiVec keyword vector
> >
> > The AltiVec header file is defining "vector", except in C++ build.
> > The keyword "vector" may conflict easily.
> > As a rule, it is better to use the alternative keyword "__vector".
> >
> > The DPDK header file rte_altivec.h takes care of undefining "vector",
> > so the applications and dependencies are free to define the name
> "vector".
> >
> > This is a compatibility breakage for applications which were using
> > the keyword "vector" for its AltiVec meaning.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> 
> Build passes with latest rdma-core master (36395896) on Ubuntu 20.04.4.

Compiler: powerpc64le-linux-gnu-gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0

> 
> Tested-by: Ali Alnubani <alialnu@nvidia.com>
> 
> Thanks,
> Ali
Ray Kinsella May 25, 2022, 11:48 a.m. UTC | #4 
Thomas Monjalon <thomas@monjalon.net> writes:

> The AltiVec header file is defining "vector", except in C++ build.
> The keyword "vector" may conflict easily.
> As a rule, it is better to use the alternative keyword "__vector".
>
> The DPDK header file rte_altivec.h takes care of undefining "vector",
> so the applications and dependencies are free to define the name "vector".
>
> This is a compatibility breakage for applications which were using
> the keyword "vector" for its AltiVec meaning.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  doc/guides/rel_notes/release_22_07.rst | 5 +++++
>  lib/eal/ppc/include/rte_altivec.h      | 7 +++++++
>  2 files changed, 12 insertions(+)
>

Acked-by: Ray Kinsella <mdr@ashroe.eu>
Thomas Monjalon May 25, 2022, 3:40 p.m. UTC | #5 
25/05/2022 13:48, Ray Kinsella:
> 
> Thomas Monjalon <thomas@monjalon.net> writes:
> 
> > The AltiVec header file is defining "vector", except in C++ build.
> > The keyword "vector" may conflict easily.
> > As a rule, it is better to use the alternative keyword "__vector".
> >
> > The DPDK header file rte_altivec.h takes care of undefining "vector",
> > so the applications and dependencies are free to define the name "vector".
> >
> > This is a compatibility breakage for applications which were using
> > the keyword "vector" for its AltiVec meaning.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >  doc/guides/rel_notes/release_22_07.rst | 5 +++++
> >  lib/eal/ppc/include/rte_altivec.h      | 7 +++++++
> >  2 files changed, 12 insertions(+)
> >
> 
> Acked-by: Ray Kinsella <mdr@ashroe.eu>

Just to make sure, we are all OK to break compatibility of rte_altivec.h?
It means the keyword vector is not available anymore with this #include.
Please confirm it is OK to merge in DPDK 22.07.
Ray Kinsella May 25, 2022, 6:02 p.m. UTC | #6 
Thomas Monjalon <thomas@monjalon.net> writes:

> 25/05/2022 13:48, Ray Kinsella:
>> 
>> Thomas Monjalon <thomas@monjalon.net> writes:
>> 
>> > The AltiVec header file is defining "vector", except in C++ build.
>> > The keyword "vector" may conflict easily.
>> > As a rule, it is better to use the alternative keyword "__vector".
>> >
>> > The DPDK header file rte_altivec.h takes care of undefining "vector",
>> > so the applications and dependencies are free to define the name "vector".
>> >
>> > This is a compatibility breakage for applications which were using
>> > the keyword "vector" for its AltiVec meaning.
>> >
>> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>> > ---
>> >  doc/guides/rel_notes/release_22_07.rst | 5 +++++
>> >  lib/eal/ppc/include/rte_altivec.h      | 7 +++++++
>> >  2 files changed, 12 insertions(+)
>> >
>> 
>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>
> Just to make sure, we are all OK to break compatibility of rte_altivec.h?
> It means the keyword vector is not available anymore with this #include.
> Please confirm it is OK to merge in DPDK 22.07.

I did think about it yes ;-).
I can't see how it would break the ABI in the short term.
And it makes sense to preclude this keyword in the long term.

So I ack'ed - did I miss something?
Tyler Retzlaff May 25, 2022, 6:34 p.m. UTC | #7 
On Wed, May 25, 2022 at 07:02:52PM +0100, Ray Kinsella wrote:
> 
> Thomas Monjalon <thomas@monjalon.net> writes:
> 
> > 25/05/2022 13:48, Ray Kinsella:
> >> 
> >> Thomas Monjalon <thomas@monjalon.net> writes:
> >> 
> >> > The AltiVec header file is defining "vector", except in C++ build.
> >> > The keyword "vector" may conflict easily.
> >> > As a rule, it is better to use the alternative keyword "__vector".
> >> >
> >> > The DPDK header file rte_altivec.h takes care of undefining "vector",
> >> > so the applications and dependencies are free to define the name "vector".
> >> >
> >> > This is a compatibility breakage for applications which were using
> >> > the keyword "vector" for its AltiVec meaning.
> >> >
> >> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> >> > ---
> >> >  doc/guides/rel_notes/release_22_07.rst | 5 +++++
> >> >  lib/eal/ppc/include/rte_altivec.h      | 7 +++++++
> >> >  2 files changed, 12 insertions(+)
> >> >
> >> 
> >> Acked-by: Ray Kinsella <mdr@ashroe.eu>
> >
> > Just to make sure, we are all OK to break compatibility of rte_altivec.h?
> > It means the keyword vector is not available anymore with this #include.
> > Please confirm it is OK to merge in DPDK 22.07.
> 
> I did think about it yes ;-).
> I can't see how it would break the ABI in the short term.
> And it makes sense to preclude this keyword in the long term.
> 
> So I ack'ed - did I miss something?

Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

double ack, impact of break is understood as follows.

* this is not an issue with abi it is an issue with api.
* the break will cause a compile failure, the action to resolve is to
  replace vector with __vector.

> 
> 
> -- 
> Regards, Ray K
Thomas Monjalon May 26, 2022, 10:18 a.m. UTC | #8 
25/05/2022 20:34, Tyler Retzlaff:
> On Wed, May 25, 2022 at 07:02:52PM +0100, Ray Kinsella wrote:
> > Thomas Monjalon <thomas@monjalon.net> writes:
> > > 25/05/2022 13:48, Ray Kinsella:
> > >> Thomas Monjalon <thomas@monjalon.net> writes:
> > >> 
> > >> > The AltiVec header file is defining "vector", except in C++ build.
> > >> > The keyword "vector" may conflict easily.
> > >> > As a rule, it is better to use the alternative keyword "__vector".
> > >> >
> > >> > The DPDK header file rte_altivec.h takes care of undefining "vector",
> > >> > so the applications and dependencies are free to define the name "vector".
> > >> >
> > >> > This is a compatibility breakage for applications which were using
> > >> > the keyword "vector" for its AltiVec meaning.
> > >> >
> > >> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > >> > ---
> > >> >  doc/guides/rel_notes/release_22_07.rst | 5 +++++
> > >> >  lib/eal/ppc/include/rte_altivec.h      | 7 +++++++
> > >> >  2 files changed, 12 insertions(+)
> > >> >
> > >> 
> > >> Acked-by: Ray Kinsella <mdr@ashroe.eu>
> > >
> > > Just to make sure, we are all OK to break compatibility of rte_altivec.h?
> > > It means the keyword vector is not available anymore with this #include.
> > > Please confirm it is OK to merge in DPDK 22.07.
> > 
> > I did think about it yes ;-).
> > I can't see how it would break the ABI in the short term.
> > And it makes sense to preclude this keyword in the long term.
> > 
> > So I ack'ed - did I miss something?
> 
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> 
> double ack, impact of break is understood as follows.
> 
> * this is not an issue with abi it is an issue with api.
> * the break will cause a compile failure, the action to resolve is to
>   replace vector with __vector.

Exactly

I'll wait few days or acks from the techboard, and will apply.
Ray Kinsella May 26, 2022, 11:02 a.m. UTC | #9 
Thomas Monjalon <thomas@monjalon.net> writes:

> 25/05/2022 20:34, Tyler Retzlaff:
>> On Wed, May 25, 2022 at 07:02:52PM +0100, Ray Kinsella wrote:
>> > Thomas Monjalon <thomas@monjalon.net> writes:
>> > > 25/05/2022 13:48, Ray Kinsella:
>> > >> Thomas Monjalon <thomas@monjalon.net> writes:
>> > >> 
>> > >> > The AltiVec header file is defining "vector", except in C++ build.
>> > >> > The keyword "vector" may conflict easily.
>> > >> > As a rule, it is better to use the alternative keyword "__vector".
>> > >> >
>> > >> > The DPDK header file rte_altivec.h takes care of undefining "vector",
>> > >> > so the applications and dependencies are free to define the name "vector".
>> > >> >
>> > >> > This is a compatibility breakage for applications which were using
>> > >> > the keyword "vector" for its AltiVec meaning.
>> > >> >
>> > >> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>> > >> > ---
>> > >> >  doc/guides/rel_notes/release_22_07.rst | 5 +++++
>> > >> >  lib/eal/ppc/include/rte_altivec.h      | 7 +++++++
>> > >> >  2 files changed, 12 insertions(+)
>> > >> >
>> > >> 
>> > >> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>> > >
>> > > Just to make sure, we are all OK to break compatibility of rte_altivec.h?
>> > > It means the keyword vector is not available anymore with this #include.
>> > > Please confirm it is OK to merge in DPDK 22.07.
>> > 
>> > I did think about it yes ;-).
>> > I can't see how it would break the ABI in the short term.
>> > And it makes sense to preclude this keyword in the long term.
>> > 
>> > So I ack'ed - did I miss something?
>> 
>> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>> 
>> double ack, impact of break is understood as follows.
>> 
>> * this is not an issue with abi it is an issue with api.
>> * the break will cause a compile failure, the action to resolve is to
>>   replace vector with __vector.
>
> Exactly
>
> I'll wait few days or acks from the techboard, and will apply.

+1
Thomas Monjalon June 1, 2022, 3:04 p.m. UTC | #10 
26/05/2022 13:02, Ray Kinsella:
> Thomas Monjalon <thomas@monjalon.net> writes:
> > 25/05/2022 20:34, Tyler Retzlaff:
> >> On Wed, May 25, 2022 at 07:02:52PM +0100, Ray Kinsella wrote:
> >> > Thomas Monjalon <thomas@monjalon.net> writes:
> >> > > 25/05/2022 13:48, Ray Kinsella:
> >> > >> Thomas Monjalon <thomas@monjalon.net> writes:
> >> > >> 
> >> > >> > The AltiVec header file is defining "vector", except in C++ build.
> >> > >> > The keyword "vector" may conflict easily.
> >> > >> > As a rule, it is better to use the alternative keyword "__vector".
> >> > >> >
> >> > >> > The DPDK header file rte_altivec.h takes care of undefining "vector",
> >> > >> > so the applications and dependencies are free to define the name "vector".
> >> > >> >
> >> > >> > This is a compatibility breakage for applications which were using
> >> > >> > the keyword "vector" for its AltiVec meaning.
> >> > >> >
> >> > >> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> >> > >> > ---
> >> > >> >  doc/guides/rel_notes/release_22_07.rst | 5 +++++
> >> > >> >  lib/eal/ppc/include/rte_altivec.h      | 7 +++++++
> >> > >> >  2 files changed, 12 insertions(+)
> >> > >> >
> >> > >> 
> >> > >> Acked-by: Ray Kinsella <mdr@ashroe.eu>
> >> > >
> >> > > Just to make sure, we are all OK to break compatibility of rte_altivec.h?
> >> > > It means the keyword vector is not available anymore with this #include.
> >> > > Please confirm it is OK to merge in DPDK 22.07.
> >> > 
> >> > I did think about it yes ;-).
> >> > I can't see how it would break the ABI in the short term.
> >> > And it makes sense to preclude this keyword in the long term.
> >> > 
> >> > So I ack'ed - did I miss something?
> >> 
> >> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> >> 
> >> double ack, impact of break is understood as follows.
> >> 
> >> * this is not an issue with abi it is an issue with api.
> >> * the break will cause a compile failure, the action to resolve is to
> >>   replace vector with __vector.
> >
> > Exactly
> >
> > I'll wait few days or acks from the techboard, and will apply.
> 
> +1

Applied
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index e49cacecef..ee60b17821 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -133,6 +133,11 @@  API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* The DPDK header file ``rte_altivec.h``,
+  which is a wrapper for the PPC header file ``altivec.h``,
+  undefines the AltiVec keyword ``vector``.
+  The alternative keyword ``__vector`` should be used instead.
+
 
 ABI Changes
 -----------
diff --git a/lib/eal/ppc/include/rte_altivec.h b/lib/eal/ppc/include/rte_altivec.h
index 1551a94544..7c088d2d16 100644
--- a/lib/eal/ppc/include/rte_altivec.h
+++ b/lib/eal/ppc/include/rte_altivec.h
@@ -9,6 +9,13 @@ 
 /* To include altivec.h, GCC version must be >= 4.8 */
 #include <altivec.h>
 
+/*
+ * The keyword "vector" is defined in altivec.h,
+ * and often conflicts with code in applications or dependencies.
+ * It is preferred to use the alternative keyword "__vector".
+ */
+#undef vector
+
 /*
  * Compilation workaround for PPC64 when AltiVec is fully enabled, e.g. std=c11.
  * Otherwise there would be a type conflict between stdbool and altivec.