From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by dpdk.org (Postfix) with ESMTP id 10A192B92 for ; Tue, 28 Aug 2018 13:44:40 +0200 (CEST) Received: by mail-wr1-f65.google.com with SMTP id n2-v6so1273699wrw.7 for ; Tue, 28 Aug 2018 04:44:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=GVqotXtx8dAIzEYYh7IqzJ4DOJooKMMHQPe7kAF0eYg=; b=EJjHa0a0HXI5L9MXBbOq3UZHkXsXVGg1XWm98XtYtZh77IbZl4zZSj7YjZTt9EUVea 1JEU0KvcfoYRKqcqvHAFbOLkumIKK1gcvXj4OXy2HiOWMMAki68LlPOCuobxqaTINubC 5zhEmBpSTPN0qmhveZDd/oOpTtFw5CD8ADnbZ9trzfKS0WuRwUySkAGuJC4wYL2MvsGG BTKolRTRisZ+FN7Fi9DFpSv7TSO2IAoJXaLAN8Eq4GiJNVJO0n7V86NwOTVHLyLrnh/i 3DK5oBpS9/+iOWnFimZEWFeESlDjXlCevRJvOZ60/zzgqUjfzJy4QsBLIQU1LjUMBpJG xs5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=GVqotXtx8dAIzEYYh7IqzJ4DOJooKMMHQPe7kAF0eYg=; b=ILqsSy6Cj/9uW/OzSJi7FXcJVg2StmXd+D4GUOlS1L2pz3jUN0tSyF59dqUBHrYOvg hP7m1m69AKzKsWyxhCmkzxAwNAdZ38g/OIAq5Mir5wmcR6oL/E7gu4cKq+cEWGc50mLY do+AwUS3PNrV3yhb22+f0NyGoIdHBxgoZ0STSrgNvomThXxlIJY10w553qL+B6DiWYVX wsldfbUQ0kvtxfJB21Np58PD0qcvyl1dhvNWQCJQBA2K47hB3Cw5PU6Li5N/netcR675 oZciR4Snd9ihRcCnrG1sqYGYcaRdQUfbvFOHEW0ZNrBZlXE3ftcnSPXroolTL8MeOham 8Q0Q== X-Gm-Message-State: APzg51D+8svYdhp2W4HHSgqXim/yqiLzszVGOLhZGnxtUWpA0Npn6aYi Hax9gxJtAQQ5n74eQ4IWm1SFXw== X-Google-Smtp-Source: ANB0Vda32SrFHJA2AWo49l5mZrsiHzW5xlf0hBEVy7deTQJLKNPAU6UhgGr97mmn2DiiI9TVSmqgbA== X-Received: by 2002:adf:9f13:: with SMTP id l19-v6mr952703wrf.206.1535456679745; Tue, 28 Aug 2018 04:44:39 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id 34-v6sm1221154wra.20.2018.08.28.04.44.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 28 Aug 2018 04:44:38 -0700 (PDT) Date: Tue, 28 Aug 2018 13:44:22 +0200 From: Adrien Mazarguil To: Christian Ehrhardt Cc: dev , Gowrishankar Muthukrishnan , Chao Zhu , Luca Boccassi , Thomas Monjalon Message-ID: <20180828114422.GG3695@6wind.com> References: <20180827122219.GB3695@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [dpdk-dev] 18.08 build error on ppc64el - bool as vector type X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Aug 2018 11:44:40 -0000 On Tue, Aug 28, 2018 at 01:30:12PM +0200, Christian Ehrhardt wrote: > On Mon, Aug 27, 2018 at 2:22 PM Adrien Mazarguil > wrote: > > > Hi Christian, > > > > On Wed, Aug 22, 2018 at 05:11:41PM +0200, Christian Ehrhardt wrote: > > > Just FYI the simple change hits similar issues later on. > > > > > > The (not really) proposed patch would have to be extended to be as > > > following. > > > We really need a better solution (or somebody has to convince me that my > > > change is better than a band aid). > > > > Thanks for reporting. I've made a quick investigation on my own and believe > > it's a toolchain issue which may affect more than this PMD; potentially all > > users of stdbool.h (C11) on this platform. > > > > Yeah I assumed as much, which is why I was hoping that some of the arch > experts would jump in and say "yeah this is a common thing and correctly > handled like " > I'll continue trying to reach out to people that should know better still > ... > > > > C11's stdbool.h defines a bool macro as _Bool (big B) along with > > true/false. On PPC targets, another file (altivec.h) defines bool as _bool > > (small b) but not true/false: > > > > #if !defined(__APPLE_ALTIVEC__) > > /* You are allowed to undef these for C++ compatibility. */ > > #define vector __vector > > #define pixel __pixel > > #define bool __bool > > #endif > > > > mlx5_nl.c explicitly includes stdbool.h to get the above definitions then > > includes mlx5.h -> rte_ether.h -> ppc_64/rte_memcpy.h -> altivec.h. > > > > For some reason the conflicting bool redefinition doesn't seem to raise any > > warnings, but results in mismatching bool and true/false definitions; an > > integer value cannot be assigned to a bool variable anymore, hence the > > build > > failure. > > > > The inability to assign integer values to bool is, in my opinion, a > > fundamental issue caused by altivec.h. If there is no way to fix this on > > the > > system, there are a couple of workarounds for DPDK, by order of preference: > > > > 1. Always #undef bool after including altivec.h in > > lib/librte_eal/common/include/arch/ppc_64/rte_memcpy.h. I do not think > > anyone expects this type to be unusable with true/false or integer > > values > > anyway. The version of altivec.h I have doesn't rely on this macro at > > all so it's probably not a big loss. > > > > The undef of a definition in header A by hedaer B can lead to most > interesting, still broken effects. > If e.g. one does > #include > #include "mlx5.h" > > or similar then it would undefine that of stdbool as well right? > In any case, the undefine not only would be suspicious it also fails right > away: > > In file included from > /home/ubuntu/deb_dpdk/lib/librte_eal/common/malloc_heap.c:27: > /home/ubuntu/deb_dpdk/lib/librte_eal/common/eal_memalloc.h:30:15: > error: unknown > type name ‘bool’; did you mean ‘_Bool’? > int socket, bool exact); > ^~~~ > _Bool > [...] > > > > > Ditto for "pixel" and "vector" keywords. Alternatively you could #define > > __APPLE_ALTIVEC__ before including altivec.h to prevent them from > > getting > > defined in the first place. > > > > Interesting I got plenty of these: > In file included from > /home/ubuntu/deb_dpdk/lib/librte_eal/common/eal_common_options.c:25: > /home/ubuntu/deb_dpdk/debian/build/static-root/include/rte_memcpy.h:39: > warning: > "__APPLE_ALTIVEC__" redefined > #define __APPLE_ALTIVEC__ > > With a few of it being even errors, but the position of the original define > is interesting. > /home/ubuntu/deb_dpdk/debian/build/static-root/include/rte_memcpy.h:39: error: > "__APPLE_ALTIVEC__" redefined [-Werror] > #define __APPLE_ALTIVEC__ > : note: this is the location of the previous definition > > So if being a built-in, shouldn't it ALWAYS be defined and never > over-declare the bool type? > > Checking GCC on the platform: > $ gcc -dM -E - < /dev/null | grep ALTI > #define __ALTIVEC__ 1 > #define __APPLE_ALTIVEC__ 1 > > > I added an #error in the header and dropped all dpdk changes. > if !defined(__APPLE_ALTIVEC__) > /* You are allowed to undef these for C++ compatibility. */ > #error WOULD REDECLARE BOOL > #define vector __vector > > And I get: > gcc -Wp,-MD,./.mlx4.o.d.tmp -Wdate-time -D_FORTIFY_SOURCE=2 -m64 -pthread > -DRTE_MACHINE_CPUFLAG_PPC64 -DRTE_MACHINE_CPUFLAG_ALTIVEC > -DRTE_MACHINE_CPUFLAG_VSX -I/home/ubuntu/deb_dpdk/debia > n/build/static-root/include -include > /home/ubuntu/deb_dpdk/debian/build/static-root/include/rte_config.h -O3 > -std=c11 -Wall -Wextra -g -I. -D_BSD_SOURCE -D_DEFAULT_SOURCE > -D_XOPEN_SOURCE=600 > -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations > -Wold-style-definition -Wpointer-arith -Wcast-align -Wnested-externs > -Wcast-qual -Wformat-nonliteral -Wformat-securi > ty -Wundef -Wwrite-strings -Wdeprecated -Wimplicit-fallthrough=2 > -Wno-format-truncation -DALLOW_EXPERIMENTAL_API -Wno-error=cast-qual > -DNDEBUG -UPEDANTIC -g -g -o mlx4.o -c /home/ubuntu/de > b_dpdk/drivers/net/mlx4/mlx4.c > In file included from > /home/ubuntu/deb_dpdk/debian/build/static-root/include/rte_memcpy.h:39, > from > /home/ubuntu/deb_dpdk/debian/build/static-root/include/rte_ether.h:21, > from > /home/ubuntu/deb_dpdk/debian/build/static-root/include/rte_ethdev.h:158, > from > /home/ubuntu/deb_dpdk/debian/build/static-root/include/rte_ethdev_driver.h:18, > > from /home/ubuntu/deb_dpdk/drivers/net/mlx4/mlx4.c:35: > /usr/lib/gcc/powerpc64le-linux-gnu/8/include/altivec.h:44:2: error: #error > WOULD REDECLARE BOOL > #error WOULD REDECLARE BOOL > > But a quick sanity test with a hello world including this special altivec > header did build not reaching the #error. > So something in DPDK undefines __ALTIVEC__ ?! > > After being that close I found which of our usual build does the switch to > trigger this. > It is "-std=c11" > > And in fact > $ gcc -std=c11 -dM -E - < /dev/null | grep ALTI > #define __ALTIVEC__ 1 > > But no __APPLE_ALTIVEC__ > > The header says > /* You are allowed to undef these for C++ compatibility. */ > > But I thought "wait a minute, didn't we just undefine it above and it > failed?" > Yes we did, and it turns out not all gcc calls have --std=c11 and due to > that it failed for those. > > > > 2. Add Altivec detection to impacted users of stdbool.h, which #undef and > > redefine bool as _Bool on their own with a short comment about broken > > toolchains. > > > > I tested a few versions of this after my findings on #1 above. > A few extra loops to jump like to make drivers/net/cxgbe/cxgbe_compat.h > usage of bool happy. > I eventually came up with the following as a fix that seems to work: > > --- 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,19 @@ > #include > /*To include altivec.h, GCC version must >= 4.8 */ > #include > +/* > + * if built with std=c11 stdbool and vector bool will conflict. > + * But if std is not c11 (which is true for some of our gcc calls) it will > + * have __APPLE_ALTIVEC__ defined which will make it not define the types > + * at all. > + * Furthermore we need to be careful to only redefine as stdbool would have > + * done if it was included - otherwise we might conflict with other > intended > + * meanings of "bool". > + */ > +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L && > defined(_STDBOOL_H) > +#undef bool > +#define bool _Bool > +#endif > > #ifdef __cplusplus > extern "C" { > > > In turn I have only checked this modification on ppc64 so far, but anyway I > still have the feeling we are only trying to poke at things with a stick > and someone with subject matter experience would just tell us. > Opinions on the change above as a "v2 RFC"? Thanks for the detailed analysis :) I'm afraid this suggestion can still break since stdbool.h won't be necessarily included before altivec.h. How about this instead? /* Blurb */ #ifndef __APPLE_ALTIVEC__ #define __APPLE_ALTIVEC__ 1 #endif #include -- Adrien Mazarguil 6WIND