From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v3] ppc64: fix compilation of when AltiVec is enabled Date: Mon, 05 Nov 2018 15:25:14 +0100 Message-ID: <6785553.SR5hTDBt0K@xps> References: <20180830115959.28935-1-christian.ehrhardt@canonical.com> <20180903092911.GU3695@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org, Christian Ehrhardt , Gowrishankar Muthukrishnan , Chao Zhu , Luca Boccassi , pradeep@us.ibm.com, Takeshi Yoshimura , dwilder@us.ibm.com To: Adrien Mazarguil Return-path: Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by dpdk.org (Postfix) with ESMTP id 3F10B4C9C for ; Mon, 5 Nov 2018 15:25:18 +0100 (CET) In-Reply-To: <20180903092911.GU3695@6wind.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 03/09/2018 11:29, Adrien Mazarguil: > Hi Christian, >=20 > Couldn't follow up on this last week, however I still have some concerns = and > comments, please see below. >=20 > On Thu, Aug 30, 2018 at 01:59:59PM +0200, Christian Ehrhardt wrote: > > The definition of almost any newer standard like --stc=3Dc11 will drop > > __APPLCE_ALTIVEC__ which otherwise would be defined. > > If that is the case then altivec.h will redefine bool to a type > > conflicting with those defined by stdbool.h. > >=20 > > This breaks compilation of 18.08 on ppc64 like: > > mlx5_nl_flow.c:407:17: error: incompatible types when assigning > > to type =E2=80=98__vector __bool int=E2=80=99 {aka =E2=80=98__vector(= 4) __bool int=E2=80=99} > > from type =E2=80=98int=E2=80=99 in_port_id_set =3D false; > >=20 > > Other alternatives were pursued on [1] but they always ended up being > > more complex than what would be appropriate for the issue we face. > >=20 > > [1]: http://mails.dpdk.org/archives/dev/2018-August/109926.html > >=20 > > Tested-by: Takeshi T Yoshimura > > Reviewed-by: Adrien Mazarguil > > Signed-off-by: Christian Ehrhardt > > --- > > .../common/include/arch/ppc_64/rte_memcpy.h | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > >=20 > > --- a/lib/librte_eal/common/include/arch/ppc_64/rte_memcpy.h > > +++ b/lib/librte_eal/common/include/arch/ppc_64/rte_memcpy.h > > @@ -37,6 +37,17 @@ > > #include > > /*To include altivec.h, GCC version must >=3D 4.8 */ > > #include > > +/* > > + * Compilation workaround for PPC64 targets when AltiVec is fully > > + * enabled e.g. with std=3Dc11. Otherwise there would be a type confli= ct > > + * of "bool" between stdbool and altivec. > > + */ > > +#if defined(__PPC64__) && !defined(__APPLE_ALTIVEC__) > > + #undef bool > > + /* redefine as in stdbool.h */ > > + #define bool _Bool > > +#endif > > + >=20 > The above will break existing C++ programs that include rte_memcpy.h. >=20 > Problem is that bool is an actual C++ type. C99 has _Bool which doesn't > exist in C++ along with a bool macro that appears only after including > stdbool.h. >=20 > To make things worse, nothing prevents C++ programs from importing a C-st= yle > bool macro by including stdbool.h (or cstdbool). >=20 > Enclosing it in #ifdef __cplusplus won't help because you never know what > bool is supposed to be in the first place as it depends on how applicatio= ns > are written. I think something like this prior suggestion [1] > (saving/restoring bool) is the only way to deal with that in a safe-ish > fashion. >=20 > Pending something better, the above #undef/#define workaround is only safe > to use inside mlx5 PMD code that triggers the compilation issue. It must = not > be found in a public header. >=20 > > #ifdef __cplusplus > > extern "C" { >=20 > [1] https://mails.dpdk.org/archives/dev/2018-August/110401.html Thank you for the review Adrien. I think we could accept the patch if some notes mention it breaks C++ applications. But... After 2 months, nobody replied or complained about the issue not fixed. So I classify this patch as rejected. Summary of alerts here: http://mails.dpdk.org/archives/dev/2018-November/118259.html