* [dpdk-stable] [dpdk-dev][PATCH] buildtools: fix pmdinfogen compilation @ 2019-07-31 6:27 pbhagavatula 2019-07-31 11:35 ` Neil Horman 2019-07-31 14:31 ` Bruce Richardson 0 siblings, 2 replies; 6+ messages in thread From: pbhagavatula @ 2019-07-31 6:27 UTC (permalink / raw) To: jerinj, Neil Horman; +Cc: dev, Pavan Nikhilesh, stable From: Pavan Nikhilesh <pbhagavatula@marvell.com> Pmdinfogen is always compiled with host gcc. If host gcc version is lessthan 7 and target gcc is greaterthan 7 pmdinfogen fails to compile due to unsupported cflags. This patch removes unsupported host cflags when the above condition is met. Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility") Cc: stable@dpdk.org Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com> --- buildtools/pmdinfogen/Makefile | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/buildtools/pmdinfogen/Makefile b/buildtools/pmdinfogen/Makefile index a97a7648f..86f883e05 100644 --- a/buildtools/pmdinfogen/Makefile +++ b/buildtools/pmdinfogen/Makefile @@ -9,6 +9,14 @@ include $(RTE_SDK)/mk/rte.vars.mk # HOSTAPP = dpdk-pmdinfogen +HOST_GCC_MAJOR = $(shell echo __GNUC__ | $(HOSTCC) -E -x c - | tail -n 1) +HOST_GCC_MINOR = $(shell echo __GNUC_MINOR__ | $(HOSTCC) -E -x c - | tail -n 1) +HOST_GCC_VERSION = $(HOST_GCC_MAJOR)$(HOST_GCC_MINOR) + +ifeq ($(shell test $(HOST_GCC_VERSION) -gt 70 && echo 1), 1) +HOST_WERROR_FLAGS = $(filter-out -Wimplicit-fallthrough=2, $(WERROR_FLAGS)) +endif + # # all sources are stored in SRCS-y # -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [dpdk-dev][PATCH] buildtools: fix pmdinfogen compilation 2019-07-31 6:27 [dpdk-stable] [dpdk-dev][PATCH] buildtools: fix pmdinfogen compilation pbhagavatula @ 2019-07-31 11:35 ` Neil Horman 2019-07-31 14:21 ` [dpdk-stable] [dpdk-dev] [PATCH] " Bruce Richardson 2019-07-31 14:31 ` Bruce Richardson 1 sibling, 1 reply; 6+ messages in thread From: Neil Horman @ 2019-07-31 11:35 UTC (permalink / raw) To: pbhagavatula; +Cc: jerinj, dev, stable On Wed, Jul 31, 2019 at 11:57:05AM +0530, pbhagavatula@marvell.com wrote: > From: Pavan Nikhilesh <pbhagavatula@marvell.com> > > Pmdinfogen is always compiled with host gcc. > If host gcc version is lessthan 7 and target gcc is greaterthan 7 > pmdinfogen fails to compile due to unsupported cflags. > This patch removes unsupported host cflags when the above condition is > met. > > Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility") > Cc: stable@dpdk.org > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com> > --- > buildtools/pmdinfogen/Makefile | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/buildtools/pmdinfogen/Makefile b/buildtools/pmdinfogen/Makefile > index a97a7648f..86f883e05 100644 > --- a/buildtools/pmdinfogen/Makefile > +++ b/buildtools/pmdinfogen/Makefile > @@ -9,6 +9,14 @@ include $(RTE_SDK)/mk/rte.vars.mk > # > HOSTAPP = dpdk-pmdinfogen > > +HOST_GCC_MAJOR = $(shell echo __GNUC__ | $(HOSTCC) -E -x c - | tail -n 1) > +HOST_GCC_MINOR = $(shell echo __GNUC_MINOR__ | $(HOSTCC) -E -x c - | tail -n 1) > +HOST_GCC_VERSION = $(HOST_GCC_MAJOR)$(HOST_GCC_MINOR) > + > +ifeq ($(shell test $(HOST_GCC_VERSION) -gt 70 && echo 1), 1) > +HOST_WERROR_FLAGS = $(filter-out -Wimplicit-fallthrough=2, $(WERROR_FLAGS)) > +endif > + A few things: 1) HOST_GCC_MAJOR and HOST_GCC_MINOR seem to already be computed in rte.toolchain-compat.mk and so I don't think you need to recompute them here 2) This seems limited in its function. That is to say, ostensibly there are simmilar incompatibilities with icc and clang that may need addressing for which there are different environment variables (CLANG_MAJOR_VERSION, etc) 3) This may also need to be reflected into the meson build environment 4) I'm not sure how this is a problem at all. In your description, you indicate that: a) pmdinfogen is always compiled with host gcc b) this fails when the target gcc uses a compiler version older than the host gcc version, leading to corresponding flag incompatibilities If (a) is true, why does (b) matter? If we are using the host gcc, then the host gcc flags should work. If we're mixing and matching host gcc and target gcc flags, that seems like a bug that should be fixed in the target rte.vars.mk Neil ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH] buildtools: fix pmdinfogen compilation 2019-07-31 11:35 ` Neil Horman @ 2019-07-31 14:21 ` Bruce Richardson 2019-07-31 14:30 ` Bruce Richardson 0 siblings, 1 reply; 6+ messages in thread From: Bruce Richardson @ 2019-07-31 14:21 UTC (permalink / raw) To: Neil Horman; +Cc: pbhagavatula, jerinj, dev, stable On Wed, Jul 31, 2019 at 07:35:03AM -0400, Neil Horman wrote: > On Wed, Jul 31, 2019 at 11:57:05AM +0530, pbhagavatula@marvell.com wrote: > > From: Pavan Nikhilesh <pbhagavatula@marvell.com> > > > > Pmdinfogen is always compiled with host gcc. > > If host gcc version is lessthan 7 and target gcc is greaterthan 7 > > pmdinfogen fails to compile due to unsupported cflags. > > This patch removes unsupported host cflags when the above condition is > > met. > > > > Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility") > > Cc: stable@dpdk.org > > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com> > > --- > > buildtools/pmdinfogen/Makefile | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/buildtools/pmdinfogen/Makefile b/buildtools/pmdinfogen/Makefile > > index a97a7648f..86f883e05 100644 > > --- a/buildtools/pmdinfogen/Makefile > > +++ b/buildtools/pmdinfogen/Makefile > > @@ -9,6 +9,14 @@ include $(RTE_SDK)/mk/rte.vars.mk > > # > > HOSTAPP = dpdk-pmdinfogen > > > > +HOST_GCC_MAJOR = $(shell echo __GNUC__ | $(HOSTCC) -E -x c - | tail -n 1) > > +HOST_GCC_MINOR = $(shell echo __GNUC_MINOR__ | $(HOSTCC) -E -x c - | tail -n 1) > > +HOST_GCC_VERSION = $(HOST_GCC_MAJOR)$(HOST_GCC_MINOR) > > + > > +ifeq ($(shell test $(HOST_GCC_VERSION) -gt 70 && echo 1), 1) > > +HOST_WERROR_FLAGS = $(filter-out -Wimplicit-fallthrough=2, $(WERROR_FLAGS)) > > +endif > > + > > A few things: > > 1) HOST_GCC_MAJOR and HOST_GCC_MINOR seem to already be computed in > rte.toolchain-compat.mk and so I don't think you need to recompute them here > > 2) This seems limited in its function. That is to say, ostensibly there are > simmilar incompatibilities with icc and clang that may need addressing for which > there are different environment variables (CLANG_MAJOR_VERSION, etc) > > 3) This may also need to be reflected into the meson build environment Initially I though that meson cross-compilation support would make this a non-issue, but looking into it further it could theoretically be a problem with meson too. Where the issue would arise is where we use "add_project_arguments()" in "config/meson.build" for the cflags. When adding those flags we only check the standard compiler, which would be the cross-compiler in the cross compilation case. Unfortunately, I don't believe that meson supports setting project options only for native or non-native compiles - it assumes that per-project arguments are really global to that whole project, and expect you to specify them per-target if not. To fix this possible issue for cross-compilation, what we really need to do in to get the native compiler too (using "meson.get_compiler('c', native: 'true')") and check that the cflags are valid for it also. In case of a flag that is supported by one compiler but not the other, the flag would be omitted, which means that instead of a compiler error we should instead get an error with reduced warning flags. (I'm assuming that it's only the warnings need adjusting here - any compiler that doesn't support "-D<define>" syntax just won't work anyway, I suspect :-)) I'll see if I can do up a quick patch to fix this for meson. > > 4) I'm not sure how this is a problem at all. In your description, you indicate > that: > a) pmdinfogen is always compiled with host gcc > b) this fails when the target gcc uses a compiler version older than the > host gcc version, leading to corresponding flag incompatibilities > If (a) is true, why does (b) matter? If we are using the host gcc, then the > host gcc flags should work. If we're mixing and matching host gcc and target > gcc flags, that seems like a bug that should be fixed in the target rte.vars.mk > Yes, this is the issue - the mixing of host and target gcc flags. /Bruce ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH] buildtools: fix pmdinfogen compilation 2019-07-31 14:21 ` [dpdk-stable] [dpdk-dev] [PATCH] " Bruce Richardson @ 2019-07-31 14:30 ` Bruce Richardson 0 siblings, 0 replies; 6+ messages in thread From: Bruce Richardson @ 2019-07-31 14:30 UTC (permalink / raw) To: Neil Horman; +Cc: pbhagavatula, jerinj, dev, stable On Wed, Jul 31, 2019 at 03:21:46PM +0100, Bruce Richardson wrote: > On Wed, Jul 31, 2019 at 07:35:03AM -0400, Neil Horman wrote: > > On Wed, Jul 31, 2019 at 11:57:05AM +0530, pbhagavatula@marvell.com wrote: > > > From: Pavan Nikhilesh <pbhagavatula@marvell.com> > > > > > > Pmdinfogen is always compiled with host gcc. > > > If host gcc version is lessthan 7 and target gcc is greaterthan 7 > > > pmdinfogen fails to compile due to unsupported cflags. > > > This patch removes unsupported host cflags when the above condition is > > > met. > > > > > > Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com> > > > --- > > > buildtools/pmdinfogen/Makefile | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/buildtools/pmdinfogen/Makefile b/buildtools/pmdinfogen/Makefile > > > index a97a7648f..86f883e05 100644 > > > --- a/buildtools/pmdinfogen/Makefile > > > +++ b/buildtools/pmdinfogen/Makefile > > > @@ -9,6 +9,14 @@ include $(RTE_SDK)/mk/rte.vars.mk > > > # > > > HOSTAPP = dpdk-pmdinfogen > > > > > > +HOST_GCC_MAJOR = $(shell echo __GNUC__ | $(HOSTCC) -E -x c - | tail -n 1) > > > +HOST_GCC_MINOR = $(shell echo __GNUC_MINOR__ | $(HOSTCC) -E -x c - | tail -n 1) > > > +HOST_GCC_VERSION = $(HOST_GCC_MAJOR)$(HOST_GCC_MINOR) > > > + > > > +ifeq ($(shell test $(HOST_GCC_VERSION) -gt 70 && echo 1), 1) > > > +HOST_WERROR_FLAGS = $(filter-out -Wimplicit-fallthrough=2, $(WERROR_FLAGS)) > > > +endif > > > + > > > > A few things: > > > > 1) HOST_GCC_MAJOR and HOST_GCC_MINOR seem to already be computed in > > rte.toolchain-compat.mk and so I don't think you need to recompute them here > > > > 2) This seems limited in its function. That is to say, ostensibly there are > > simmilar incompatibilities with icc and clang that may need addressing for which > > there are different environment variables (CLANG_MAJOR_VERSION, etc) > > > > 3) This may also need to be reflected into the meson build environment > > Initially I though that meson cross-compilation support would make this a > non-issue, but looking into it further it could theoretically be a problem > with meson too. Where the issue would arise is where we use > "add_project_arguments()" in "config/meson.build" for the cflags. When > adding those flags we only check the standard compiler, which would be the > cross-compiler in the cross compilation case. Unfortunately, I don't > believe that meson supports setting project options only for native or > non-native compiles - it assumes that per-project arguments are really > global to that whole project, and expect you to specify them per-target if > not. > > To fix this possible issue for cross-compilation, what we really need to do > in to get the native compiler too (using "meson.get_compiler('c', native: > 'true')") and check that the cflags are valid for it also. In case of a > flag that is supported by one compiler but not the other, the flag would be > omitted, which means that instead of a compiler error we should instead get > an error with reduced warning flags. (I'm assuming that it's only the > warnings need adjusting here - any compiler that doesn't support > "-D<define>" syntax just won't work anyway, I suspect :-)) > > I'll see if I can do up a quick patch to fix this for meson. > Actually, looks like I sent the reply too quickly and I may be incorrect here. Checking the build.ninja file for a cross-build for arm64-armv8, I already see different flags for pmdinfogen and other objects, so looks to me like no issue after all. [This is with meson v0.51]. /Bruce ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH] buildtools: fix pmdinfogen compilation 2019-07-31 6:27 [dpdk-stable] [dpdk-dev][PATCH] buildtools: fix pmdinfogen compilation pbhagavatula 2019-07-31 11:35 ` Neil Horman @ 2019-07-31 14:31 ` Bruce Richardson 2019-08-02 3:35 ` [dpdk-stable] [EXT] " Pavan Nikhilesh Bhagavatula 1 sibling, 1 reply; 6+ messages in thread From: Bruce Richardson @ 2019-07-31 14:31 UTC (permalink / raw) To: pbhagavatula; +Cc: jerinj, Neil Horman, dev, stable On Wed, Jul 31, 2019 at 11:57:05AM +0530, pbhagavatula@marvell.com wrote: > From: Pavan Nikhilesh <pbhagavatula@marvell.com> > > Pmdinfogen is always compiled with host gcc. > If host gcc version is lessthan 7 and target gcc is greaterthan 7 > pmdinfogen fails to compile due to unsupported cflags. > This patch removes unsupported host cflags when the above condition is > met. > > Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen utility") > Cc: stable@dpdk.org > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com> > --- Can you perhaps check with a meson cross-compile and see if you encounter the same issue? If so, I'll investigate. Thanks, /Bruce ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-stable] [EXT] Re: [dpdk-dev] [PATCH] buildtools: fix pmdinfogen compilation 2019-07-31 14:31 ` Bruce Richardson @ 2019-08-02 3:35 ` Pavan Nikhilesh Bhagavatula 0 siblings, 0 replies; 6+ messages in thread From: Pavan Nikhilesh Bhagavatula @ 2019-08-02 3:35 UTC (permalink / raw) To: Bruce Richardson; +Cc: Jerin Jacob Kollanukkaran, Neil Horman, dev, stable Hi Bruce, Neil, We had this patch in our internal tree that was intended to fix -Wimplicit-fallthrough related error due to Host and Target GCC mismatch. But I just verified both combination of GCC in Make and Meson seems like it has already been addressed somewhere down the line. So, I'm dropping the patch. Regards, Pavan. >-----Original Message----- >From: Bruce Richardson <bruce.richardson@intel.com> >Sent: Wednesday, July 31, 2019 8:02 PM >To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com> >Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Neil Horman ><nhorman@tuxdriver.com>; dev@dpdk.org; stable@dpdk.org >Subject: [EXT] Re: [dpdk-dev] [PATCH] buildtools: fix pmdinfogen >compilation > >External Email > >---------------------------------------------------------------------- >On Wed, Jul 31, 2019 at 11:57:05AM +0530, pbhagavatula@marvell.com >wrote: >> From: Pavan Nikhilesh <pbhagavatula@marvell.com> >> >> Pmdinfogen is always compiled with host gcc. >> If host gcc version is lessthan 7 and target gcc is greaterthan 7 >> pmdinfogen fails to compile due to unsupported cflags. >> This patch removes unsupported host cflags when the above >condition is >> met. >> >> Fixes: 98b0fdb0ffc6 ("pmdinfogen: add buildtools and pmdinfogen >utility") >> Cc: stable@dpdk.org >> >> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com> >> --- > >Can you perhaps check with a meson cross-compile and see if you >encounter >the same issue? If so, I'll investigate. > >Thanks, >/Bruce ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-02 3:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-31 6:27 [dpdk-stable] [dpdk-dev][PATCH] buildtools: fix pmdinfogen compilation pbhagavatula 2019-07-31 11:35 ` Neil Horman 2019-07-31 14:21 ` [dpdk-stable] [dpdk-dev] [PATCH] " Bruce Richardson 2019-07-31 14:30 ` Bruce Richardson 2019-07-31 14:31 ` Bruce Richardson 2019-08-02 3:35 ` [dpdk-stable] [EXT] " Pavan Nikhilesh Bhagavatula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).