* [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-dev] [PATCH] buildtools: fix pmdinfogen compilation
2019-07-31 6:27 [dpdk-dev] [PATCH] buildtools: fix pmdinfogen compilation pbhagavatula
@ 2019-07-31 11:35 ` Neil Horman
2019-07-31 14:21 ` 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-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-dev] [PATCH] buildtools: fix pmdinfogen compilation
2019-07-31 14:21 ` 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-dev] [PATCH] buildtools: fix pmdinfogen compilation
2019-07-31 6:27 [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-dev] [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-dev] [EXT] Re: [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-dev] [PATCH] buildtools: fix pmdinfogen compilation pbhagavatula
2019-07-31 11:35 ` Neil Horman
2019-07-31 14:21 ` Bruce Richardson
2019-07-31 14:30 ` Bruce Richardson
2019-07-31 14:31 ` Bruce Richardson
2019-08-02 3:35 ` [dpdk-dev] [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).