> On Nov 6, 2018, at 9:30 PM, Yongseok Koh <ys...@mellanox.com> wrote:
> 
> 
>> On Nov 5, 2018, at 6:06 AM, Wiles, Keith <keith.wi...@intel.com> wrote:
>> 
>> 
>> 
>>> On Nov 2, 2018, at 9:04 PM, Yongseok Koh <ys...@mellanox.com> wrote:
>>> 
>>> This is a workaround to prevent a crash, which might be caused by
>>> optimization of newer gcc (7.3.0) on Intel Skylake.
>> 
>> Should the code below not also test for the gcc version and the Sky Lake 
>> processor, maybe I am wrong but it seems it is turning AVX512 for all GCC 
>> builds
> 
> I didn't want to check gcc version as 7.3.0 is very new. Only gcc 8 is newly 
> up since then (gcc 8.2).
> Also, I wasn't able to test every gcc versions and I wanted to be a bit 
> conservative for this crash.
> Performance drop (if any) by disabling a new (experimental) feature would be 
> less risky than unaccountable crash.
> And, it does disable the feature only if CONFIG_RTE_ENABLE_AVX512=n. Please 
> refer to v3.

Are you not turning off all of the GCC versions for AVX512. And you can test 
for range or greater then GCC version and it just seems like we are turning off 
every gcc version, is that true?
> 
>> 
>> Also bug 97 seems a bit obscure reference, maybe you know the bug report, 
>> but more details would be good?
> 
> I sent out the report to dev list two month ago. And I created the Bug 97 in 
> order to reference it in the commit message.
> I didn't want to repeat same message here and there, but it would've been 
> better to have some sort of summary of the Bug, although v3 has a few more 
> words. However, v3 has been merged.

Still this is too obscure if nothing else give a link to a specific bug not 
just 97.
> 
> Thanks,
> Yongseok
> 
>>> 
>>> Bugzilla ID: 97
>>> 
>>> Cc: sta...@dpdk.org
>>> 
>>> Signed-off-by: Yongseok Koh <ys...@mellanox.com>
>>> ---
>>> 
>>> v2:
>>> * disable the flag only in case of gcc
>>> 
>>> config/x86/meson.build | 5 +++++
>>> mk/rte.cpuflags.mk     | 7 +++++++
>>> 2 files changed, 12 insertions(+)
>>> 
>>> diff --git a/config/x86/meson.build b/config/x86/meson.build
>>> index 33efb5e547..8ddca0ea9f 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 AVX512F support of gcc as a workaround for Bug 97
>>> +   if cc.get_id() == 'gcc' and 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..a8c26fb011 100644
>>> --- a/mk/rte.cpuflags.mk
>>> +++ b/mk/rte.cpuflags.mk
>>> @@ -68,6 +68,13 @@ endif
>>> 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)
>>> +   ifeq ($(shell $(CC) --target-help | grep -q mavx512f && echo 1), 1)
>>> +           MACHINE_CFLAGS += -mno-avx512f
>>> +   endif
>>> +endif
>>> endif
>>> endif
>>> 
>>> -- 
>>> 2.11.0
>>> 
>> 
>> Regards,
>> Keith
> 

Regards,
Keith

Reply via email to