diff mbox series

[v3] mk: fix scope of disabling AVX512F support

Message ID 20190103162313.85623-1-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v3] mk: fix scope of disabling AVX512F support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Ferruh Yigit Jan. 3, 2019, 4:23 p.m. UTC 
  AVX512 was disabled for GCC because of Bugzilla issue 97 [1],
the GCC defect submitted for the issue [2] highlighted that this is
a known binutils version 2.30 issue.

Narrowed the scope of no-avx512 to the this specific binutils version.

[1]
https://bugs.dpdk.org/show_bug.cgi?id=97

[2]
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88096

Fixes: 8d07c82b239f ("mk: disable gcc AVX512F support")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Tom Barbette <barbette@kth.se>
Cc: Thomas Monjalon <thomas@monjalon.net>
Cc: Yongseok Koh <yskoh@mellanox.com>
Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>

v2:
* Added warning message (print only once for eal)
* Moved decision details to compat file, kept execution in cpuflags

v3:
* replace 'ld' with '$(LD)' for cross build
* added meson support
---
 config/meson.build                       |  8 ++++++++
 mk/rte.cpuflags.mk                       |  4 ++--
 mk/toolchain/gcc/rte.toolchain-compat.mk | 10 ++++++++++
 3 files changed, 20 insertions(+), 2 deletions(-)

Comments

Yongseok Koh Jan. 3, 2019, 10:46 p.m. UTC | #1 
> On Jan 3, 2019, at 8:23 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> AVX512 was disabled for GCC because of Bugzilla issue 97 [1],
> the GCC defect submitted for the issue [2] highlighted that this is
> a known binutils version 2.30 issue.
> 
> Narrowed the scope of no-avx512 to the this specific binutils version.
> 
> [1]
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.dpdk.org%2Fshow_bug.cgi%3Fid%3D97&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C9e8d0eb500ca4cb74c1508d67197c6b0%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636821294022213177&amp;sdata=Rxj7HxBWfp4hMzfRkiGtnSYt8N0TG9xnAL0ZYBpYYa0%3D&amp;reserved=0
> 
> [2]
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D88096&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C9e8d0eb500ca4cb74c1508d67197c6b0%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636821294022213177&amp;sdata=EqzsamoYfFve5JAsgyWw51JOBhhiKTOCrnfZoYTGN28%3D&amp;reserved=0
> 
> Fixes: 8d07c82b239f ("mk: disable gcc AVX512F support")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thanks

> Cc: Tom Barbette <barbette@kth.se>
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Yongseok Koh <yskoh@mellanox.com>
> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Cc: Bruce Richardson <bruce.richardson@intel.com>
> 
> v2:
> * Added warning message (print only once for eal)
> * Moved decision details to compat file, kept execution in cpuflags
> 
> v3:
> * replace 'ld' with '$(LD)' for cross build
> * added meson support
> ---
> config/meson.build                       |  8 ++++++++
> mk/rte.cpuflags.mk                       |  4 ++--
> mk/toolchain/gcc/rte.toolchain-compat.mk | 10 ++++++++++
> 3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/config/meson.build b/config/meson.build
> index db32499b3..40802fc88 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -43,6 +43,14 @@ toolchain = cc.get_id()
> dpdk_conf.set_quoted('RTE_TOOLCHAIN', toolchain)
> dpdk_conf.set('RTE_TOOLCHAIN_' + toolchain.to_upper(), 1)
> 
> +# get binutils version for the workaround of Bug 97
> +ldver = run_command('ld', '-v').stdout().strip()
> +if ldver.contains('2.30')
> +	if cc.has_argument('-mno-avx512f')
> +		machine_args += '-mno-avx512f'
> +	endif
> +endif
> +
> add_project_link_arguments('-Wl,--no-as-needed', language: 'c')
> dpdk_extra_ldflags += '-Wl,--no-as-needed'
> 
> diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk
> index c3291b17a..541211c61 100644
> --- a/mk/rte.cpuflags.mk
> +++ b/mk/rte.cpuflags.mk
> @@ -69,8 +69,8 @@ ifneq ($(filter $(AUTO_CPUFLAGS),__AVX512F__),)
> ifeq ($(CONFIG_RTE_ENABLE_AVX512),y)
> CPUFLAGS += AVX512F
> else
> -# disable AVX512F support of gcc as a workaround for Bug 97
> -ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
> +# disable AVX512F support for GCC & binutils 2.30 as a workaround for Bug 97
> +ifeq ($(FORCE_DISABLE_AVX512),y)
> MACHINE_CFLAGS += -mno-avx512f
> endif
> endif
> diff --git a/mk/toolchain/gcc/rte.toolchain-compat.mk b/mk/toolchain/gcc/rte.toolchain-compat.mk
> index 44904295c..33ea3f03a 100644
> --- a/mk/toolchain/gcc/rte.toolchain-compat.mk
> +++ b/mk/toolchain/gcc/rte.toolchain-compat.mk
> @@ -20,6 +20,16 @@ HOST_GCC_MINOR = $(shell echo __GNUC_MINOR__ | $(HOSTCC) -E -x c - | tail -n 1)
> HOST_GCC_PATCHLEVEL = $(shell echo __GNUC_PATCHLEVEL__ | $(HOSTCC) -E -x c - | tail -n 1)
> HOST_GCC_VERSION = $(HOST_GCC_MAJOR)$(HOST_GCC_MINOR)
> 
> +LD_VERSION = $(shell $(LD) -v)
> +# disable AVX512F support for GCC & binutils 2.30 as a workaround for Bug 97
> +ifneq ($(filter 2.30%,$(LD_VERSION)),)
> +FORCE_DISABLE_AVX512 := y
> +# print warning only once for librte_eal
> +ifneq ($(filter %librte_eal,$(lastword $(CURDIR))),)
> +$(warning AVX512 support disabled because of ld 2.30. See Bug 97)
> +endif
> +endif
> +
> # if GCC is older than 4.x
> ifeq ($(shell test $(GCC_VERSION) -lt 40 && echo 1), 1)
> 	MACHINE_CFLAGS =
> -- 
> 2.17.2
>
Varghese, Vipin Jan. 4, 2019, 2:40 a.m. UTC | #2 
Hi Ferruh,

Should not be there a documentation update in 'Known Issues and Limitations in Legacy Releases' for the limitation. If it is already added can you please add the patchwork link for the same.

Thanks
Vipin Varghese

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Yongseok Koh
> Sent: Friday, January 4, 2019 4:17 AM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; dev <dev@dpdk.org>; dpdk
> stable <stable@dpdk.org>; Tom Barbette <barbette@kth.se>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3] mk: fix scope of disabling AVX512F support
> 
> 
> > On Jan 3, 2019, at 8:23 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >
> > AVX512 was disabled for GCC because of Bugzilla issue 97 [1], the GCC
> > defect submitted for the issue [2] highlighted that this is a known
> > binutils version 2.30 issue.
> >
> > Narrowed the scope of no-avx512 to the this specific binutils version.
> >
> > [1]
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbug
> >
> s.dpdk.org%2Fshow_bug.cgi%3Fid%3D97&amp;data=02%7C01%7Cyskoh%40
> mellano
> >
> x.com%7C9e8d0eb500ca4cb74c1508d67197c6b0%7Ca652971c7d2e4d9ba6a4
> d149256
> >
> f461b%7C0%7C0%7C636821294022213177&amp;sdata=Rxj7HxBWfp4hMzfRki
> GtnSYt8
> > N0TG9xnAL0ZYBpYYa0%3D&amp;reserved=0
> >
> > [2]
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc
> >
> .gnu.org%2Fbugzilla%2Fshow_bug.cgi%3Fid%3D88096&amp;data=02%7C01%
> 7Cysk
> >
> oh%40mellanox.com%7C9e8d0eb500ca4cb74c1508d67197c6b0%7Ca652971c
> 7d2e4d9
> >
> ba6a4d149256f461b%7C0%7C0%7C636821294022213177&amp;sdata=Eqzsa
> moYfFve5
> > JAsgyWw51JOBhhiKTOCrnfZoYTGN28%3D&amp;reserved=0
> >
> > Fixes: 8d07c82b239f ("mk: disable gcc AVX512F support")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> Acked-by: Yongseok Koh <yskoh@mellanox.com>
> 
> Thanks
> 
> > Cc: Tom Barbette <barbette@kth.se>
> > Cc: Thomas Monjalon <thomas@monjalon.net>
> > Cc: Yongseok Koh <yskoh@mellanox.com>
> > Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > Cc: Bruce Richardson <bruce.richardson@intel.com>
> >
> > v2:
> > * Added warning message (print only once for eal)
> > * Moved decision details to compat file, kept execution in cpuflags
> >
> > v3:
> > * replace 'ld' with '$(LD)' for cross build
> > * added meson support
> > ---
> > config/meson.build                       |  8 ++++++++
> > mk/rte.cpuflags.mk                       |  4 ++--
> > mk/toolchain/gcc/rte.toolchain-compat.mk | 10 ++++++++++
> > 3 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/config/meson.build b/config/meson.build index
> > db32499b3..40802fc88 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -43,6 +43,14 @@ toolchain = cc.get_id()
> > dpdk_conf.set_quoted('RTE_TOOLCHAIN', toolchain)
> > dpdk_conf.set('RTE_TOOLCHAIN_' + toolchain.to_upper(), 1)
> >
> > +# get binutils version for the workaround of Bug 97 ldver =
> > +run_command('ld', '-v').stdout().strip() if ldver.contains('2.30')
> > +	if cc.has_argument('-mno-avx512f')
> > +		machine_args += '-mno-avx512f'
> > +	endif
> > +endif
> > +
> > add_project_link_arguments('-Wl,--no-as-needed', language: 'c')
> > dpdk_extra_ldflags += '-Wl,--no-as-needed'
> >
> > diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk index
> > c3291b17a..541211c61 100644
> > --- a/mk/rte.cpuflags.mk
> > +++ b/mk/rte.cpuflags.mk
> > @@ -69,8 +69,8 @@ ifneq ($(filter $(AUTO_CPUFLAGS),__AVX512F__),) ifeq
> > ($(CONFIG_RTE_ENABLE_AVX512),y) CPUFLAGS += AVX512F else -# disable
> > AVX512F support of gcc as a workaround for Bug 97 -ifeq
> > ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
> > +# disable AVX512F support for GCC & binutils 2.30 as a workaround for
> > +Bug 97 ifeq ($(FORCE_DISABLE_AVX512),y)
> > MACHINE_CFLAGS += -mno-avx512f
> > endif
> > endif
> > diff --git a/mk/toolchain/gcc/rte.toolchain-compat.mk
> > b/mk/toolchain/gcc/rte.toolchain-compat.mk
> > index 44904295c..33ea3f03a 100644
> > --- a/mk/toolchain/gcc/rte.toolchain-compat.mk
> > +++ b/mk/toolchain/gcc/rte.toolchain-compat.mk
> > @@ -20,6 +20,16 @@ HOST_GCC_MINOR = $(shell echo __GNUC_MINOR__
> |
> > $(HOSTCC) -E -x c - | tail -n 1) HOST_GCC_PATCHLEVEL = $(shell echo
> > __GNUC_PATCHLEVEL__ | $(HOSTCC) -E -x c - | tail -n 1)
> > HOST_GCC_VERSION = $(HOST_GCC_MAJOR)$(HOST_GCC_MINOR)
> >
> > +LD_VERSION = $(shell $(LD) -v)
> > +# disable AVX512F support for GCC & binutils 2.30 as a workaround for
> > +Bug 97 ifneq ($(filter 2.30%,$(LD_VERSION)),)
> > +FORCE_DISABLE_AVX512 := y
> > +# print warning only once for librte_eal ifneq ($(filter
> > +%librte_eal,$(lastword $(CURDIR))),) $(warning AVX512 support
> > +disabled because of ld 2.30. See Bug 97) endif endif
> > +
> > # if GCC is older than 4.x
> > ifeq ($(shell test $(GCC_VERSION) -lt 40 && echo 1), 1)
> > 	MACHINE_CFLAGS =
> > --
> > 2.17.2
> >
Ferruh Yigit Jan. 4, 2019, 10:27 a.m. UTC | #3 
On 1/4/2019 2:40 AM, Varghese, Vipin wrote:
> Hi Ferruh,
> 
> Should not be there a documentation update in 'Known Issues and Limitations in Legacy Releases' for the limitation. If it is already added can you please add the patchwork link for the same.

It is good idea to document this limitation.
Previous limitation was documented in 18.11 release notes, I can move it into
known_issues.rst, and put updated limitation into 19.02 release notes.

> 
> Thanks
> Vipin Varghese
Varghese, Vipin Jan. 4, 2019, 10:28 a.m. UTC | #4 
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, January 4, 2019 3:58 PM
> To: Varghese, Vipin <vipin.varghese@intel.com>; Yongseok Koh
> <yskoh@mellanox.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; dev <dev@dpdk.org>; dpdk
> stable <stable@dpdk.org>; Tom Barbette <barbette@kth.se>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: Re: [PATCH v3] mk: fix scope of disabling AVX512F support
> 
> On 1/4/2019 2:40 AM, Varghese, Vipin wrote:
> > Hi Ferruh,
> >
> > Should not be there a documentation update in 'Known Issues and
> Limitations in Legacy Releases' for the limitation. If it is already added can you
> please add the patchwork link for the same.
> 
> It is good idea to document this limitation.
> Previous limitation was documented in 18.11 release notes, I can move it into
> known_issues.rst, and put updated limitation into 19.02 release notes.
Thanks Ferruh for understanding

> 
> >
> > Thanks
> > Vipin Varghese
>
diff mbox series

Patch

diff --git a/config/meson.build b/config/meson.build
index db32499b3..40802fc88 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -43,6 +43,14 @@  toolchain = cc.get_id()
 dpdk_conf.set_quoted('RTE_TOOLCHAIN', toolchain)
 dpdk_conf.set('RTE_TOOLCHAIN_' + toolchain.to_upper(), 1)
 
+# get binutils version for the workaround of Bug 97
+ldver = run_command('ld', '-v').stdout().strip()
+if ldver.contains('2.30')
+	if cc.has_argument('-mno-avx512f')
+		machine_args += '-mno-avx512f'
+	endif
+endif
+
 add_project_link_arguments('-Wl,--no-as-needed', language: 'c')
 dpdk_extra_ldflags += '-Wl,--no-as-needed'
 
diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk
index c3291b17a..541211c61 100644
--- a/mk/rte.cpuflags.mk
+++ b/mk/rte.cpuflags.mk
@@ -69,8 +69,8 @@  ifneq ($(filter $(AUTO_CPUFLAGS),__AVX512F__),)
 ifeq ($(CONFIG_RTE_ENABLE_AVX512),y)
 CPUFLAGS += AVX512F
 else
-# disable AVX512F support of gcc as a workaround for Bug 97
-ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
+# disable AVX512F support for GCC & binutils 2.30 as a workaround for Bug 97
+ifeq ($(FORCE_DISABLE_AVX512),y)
 MACHINE_CFLAGS += -mno-avx512f
 endif
 endif
diff --git a/mk/toolchain/gcc/rte.toolchain-compat.mk b/mk/toolchain/gcc/rte.toolchain-compat.mk
index 44904295c..33ea3f03a 100644
--- a/mk/toolchain/gcc/rte.toolchain-compat.mk
+++ b/mk/toolchain/gcc/rte.toolchain-compat.mk
@@ -20,6 +20,16 @@  HOST_GCC_MINOR = $(shell echo __GNUC_MINOR__ | $(HOSTCC) -E -x c - | tail -n 1)
 HOST_GCC_PATCHLEVEL = $(shell echo __GNUC_PATCHLEVEL__ | $(HOSTCC) -E -x c - | tail -n 1)
 HOST_GCC_VERSION = $(HOST_GCC_MAJOR)$(HOST_GCC_MINOR)
 
+LD_VERSION = $(shell $(LD) -v)
+# disable AVX512F support for GCC & binutils 2.30 as a workaround for Bug 97
+ifneq ($(filter 2.30%,$(LD_VERSION)),)
+FORCE_DISABLE_AVX512 := y
+# print warning only once for librte_eal
+ifneq ($(filter %librte_eal,$(lastword $(CURDIR))),)
+$(warning AVX512 support disabled because of ld 2.30. See Bug 97)
+endif
+endif
+
 # if GCC is older than 4.x
 ifeq ($(shell test $(GCC_VERSION) -lt 40 && echo 1), 1)
 	MACHINE_CFLAGS =