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 23:31:37 +0000 Message-ID: <20181102233115.GD15737@mtidpdk.mti.labs.mlnx> References: <20181023212318.43082-1-yskoh@mellanox.com> <3a34ea82-fbdf-2ebd-c6d9-9713bfadefb8@intel.com> <20181102205926.GA15737@mtidpdk.mti.labs.mlnx> 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: 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 09:46:09PM +0000, Ferruh Yigit wrote: > On 11/2/2018 8:59 PM, Yongseok Koh wrote: > > 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 > >>> > >>> After checking the defect description again, this is the issue observ= ed in > >>> rte_memcpy() implementation for AVX2, compiler uses AVX512F instructi= ons while > >>> compiling it which causes the failure, so this may be a compiler defe= ct but we > >>> don't know the root cause yet. > >> > >> 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? > >> > >>> > >>> 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 fo= r rc2. > >>> > >>> What this patch does is to prevent compiler to use avx512f instructio= n when > >>> "CONFIG_RTE_ENABLE_AVX512=3Dn". > >>> > >>> Concern is this will affect all DPDK generated code for x86, but sinc= e > >>> 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 pa= tch for > >>> now. Please find below comment. > >>> > >>>> > >>>> 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) > >>> > >>> This will not work for ICC, and do we need this? AUTO_CPUFLAGS alread= y should > >>> have what you are looking for, so I think this check can be removed. > >=20 > > This is different from AUTO_CPUFLAGS as it tries to check compiler flag= support. >=20 > What AUTO_CPUFLAGS does? >=20 > It is output of `cc -march=3Dxxx -dM -E - < /dev/null`, which list define= d macros > for that specific march provided. >=20 > Like if you use `-march=3Dcorei7` you won't see __AVX2__ set. >=20 > And for `native`, if compiler doesn't support AVX2, I assume it won't abl= e to > output __AVX2__ >=20 > Is there a case AUTO_CPUFLAGS has __AVX512F__ but "$(CC) --target-help" d= oesn't > have `mavx512f`? Right. For some reason, I have wrong impression that the flag grabs cpuflag= s of the compile host. Will submit a new version. > > And per your question, I have only tested it with gcc, so I agree on ap= plying it > > only for gcc. Will submit v2. But, I don't think we need to check gcc v= ersion as > > there's no fix reported yet in a newer gcc version and this patch would= have > > very limited impact. avx512f support is quite new and kinda experimenta= l so > > far. Dropping a bit of performance would be better than crash. :-) > >=20 > > Thanks for your review, > > Yongseok > >=20 > >>>> +MACHINE_CFLAGS +=3D -mno-avx512f > >>>> +endif > >>>> endif > >>>> endif > >>>> =20 > >>>> > >>> > >> >=20