Re: [dpdk-dev] [dpdk-stable] [PATCH] build: disable compiler AVX512F support

[Ferruh Yigit <ferruh.yigit@intel.com>]

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 observed in
> rte_memcpy() implementation for AVX2, compiler uses AVX512F instructions while
> compiling it which causes the failure, so this may be a compiler defect 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 for rc2.
> 
> What this patch does is to prevent compiler to use avx512f instruction when
> "CONFIG_RTE_ENABLE_AVX512=n".
> 
> 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 patch for
> now. Please find below comment.
> 
>>
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>>  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) != ''
>>  	dpdk_conf.set('RTE_MACHINE_CPUFLAG_AVX512F', 1)
>>  	compile_time_cpuflags += ['RTE_CPUFLAG_AVX512F']
>> +else
>> +# disable compiler's AVX512F support as a workaround for Bug 97
>> +	if cc.has_argument('-mavx512f')
>> +		machine_args += '-mno-avx512f'
>> +	endif
>>  endif
>>  
>>  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 += 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 already should
> have what you are looking for, so I think this check can be removed.
> 
>> +MACHINE_CFLAGS += -mno-avx512f
>> +endif
>>  endif
>>  endif
>>  
>>
>