From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yongseok Koh Subject: Re: [dpdk-stable] [PATCH] build: disable compiler AVX512F support Date: Fri, 2 Nov 2018 20:59:37 +0000 Message-ID: <20181102205926.GA15737@mtidpdk.mti.labs.mlnx> References: <20181023212318.43082-1-yskoh@mellanox.com> <3a34ea82-fbdf-2ebd-c6d9-9713bfadefb8@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: Thomas Monjalon , "bruce.richardson@intel.com" , "dev@dpdk.org" , Shahaf Shuler , "stable@dpdk.org" , Konstantin Ananyev , Anatoly Burakov To: Ferruh Yigit Return-path: In-Reply-To: <3a34ea82-fbdf-2ebd-c6d9-9713bfadefb8@intel.com> Content-Language: en-US Content-ID: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Fri, Nov 02, 2018 at 01:48:11PM +0000, Ferruh Yigit wrote: > On 11/2/2018 12:42 PM, Ferruh Yigit wrote: > > On 10/23/2018 10:23 PM, Yongseok Koh wrote: > >> This is a workaround to prevent a crash, which might be caused by > >> optimization of newer gcc (7.3.0) on Intel Skylake. > >> > >> Bugzilla ID: 97 > >=20 > > After checking the defect description again, this is the issue observed= in > > rte_memcpy() implementation for AVX2, compiler uses AVX512F instruction= s while > > compiling it which causes the failure, so this may be a compiler defect= but we > > don't know the root cause yet. >=20 > Is the issue only with gcc, and only with specific version of gcc? > If so can we reduce the disabling avx512 only to that gcc version? >=20 > >=20 > > I think best solution is to find the root cause and fix either avx2 > > implementation or compiler, but this seems won't be soon, at least for = rc2. > >=20 > > What this patch does is to prevent compiler to use avx512f instruction = when > > "CONFIG_RTE_ENABLE_AVX512=3Dn". > >=20 > > Concern is this will affect all DPDK generated code for x86, but since > > rte_memcpy() in header file there is no way to disable using avx512f > > instructions locally for rte_memcpy(). > > I can't think of any other solution for now, so OK to go with this patc= h for > > now. Please find below comment. > >=20 > >> > >> Cc: stable@dpdk.org > >> > >> Signed-off-by: Yongseok Koh > >> --- > >> config/x86/meson.build | 5 +++++ > >> mk/rte.cpuflags.mk | 5 +++++ > >> 2 files changed, 10 insertions(+) > >> > >> diff --git a/config/x86/meson.build b/config/x86/meson.build > >> index 33efb5e547..e10ba872ac 100644 > >> --- a/config/x86/meson.build > >> +++ b/config/x86/meson.build > >> @@ -47,6 +47,11 @@ endif > >> if cc.get_define('__AVX512F__', args: march_opt) !=3D '' > >> dpdk_conf.set('RTE_MACHINE_CPUFLAG_AVX512F', 1) > >> compile_time_cpuflags +=3D ['RTE_CPUFLAG_AVX512F'] > >> +else > >> +# disable compiler's AVX512F support as a workaround for Bug 97 > >> + if cc.has_argument('-mavx512f') > >> + machine_args +=3D '-mno-avx512f' > >> + endif > >> endif > >> =20 > >> dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64) > >> diff --git a/mk/rte.cpuflags.mk b/mk/rte.cpuflags.mk > >> index 43ed84155b..8fdb0cc2c3 100644 > >> --- a/mk/rte.cpuflags.mk > >> +++ b/mk/rte.cpuflags.mk > >> @@ -68,6 +68,11 @@ endif > >> ifneq ($(filter $(AUTO_CPUFLAGS),__AVX512F__),) > >> ifeq ($(CONFIG_RTE_ENABLE_AVX512),y) > >> CPUFLAGS +=3D AVX512F > >> +else > >> +# disable compiler's AVX512F support as a workaround for Bug 97 > >> +ifeq ($(shell $(CC) --target-help | grep -q mavx512f && echo 1), 1) > >=20 > > This will not work for ICC, and do we need this? AUTO_CPUFLAGS already = should > > have what you are looking for, so I think this check can be removed. This is different from AUTO_CPUFLAGS as it tries to check compiler flag sup= port. And per your question, I have only tested it with gcc, so I agree on applyi= ng it only for gcc. Will submit v2. But, I don't think we need to check gcc versi= on as there's no fix reported yet in a newer gcc version and this patch would hav= e very limited impact. avx512f support is quite new and kinda experimental so far. Dropping a bit of performance would be better than crash. :-) Thanks for your review, Yongseok > >> +MACHINE_CFLAGS +=3D -mno-avx512f > >> +endif > >> endif > >> endif > >> =20 > >> > >=20 >=20