[dpdk-dev] [PATCH v2 07/12] acl: add infrastructure to support AVX512 classify

[Medvedkin, Vladimir]

Hi Bruce,

On 16/09/2020 10:11, Bruce Richardson wrote:
> On Tue, Sep 15, 2020 at 05:50:20PM +0100, Konstantin Ananyev wrote:
>> Add necessary changes to support new AVX512 specific ACL classify
>> algorithm:
>>   - changes in meson.build to check that build tools
>>     (compiler, assembler, etc.) do properly support AVX512.
>>   - run-time checks to make sure target platform does support AVX512.
>>   - dummy rte_acl_classify_avx512() for targets where AVX512
>>     implementation couldn't be properly supported.
>>
>> Signed-off-by: Konstantin Ananyev <konstantin.ananyev at intel.com>
>> ---
> 
> This all looks correct, though I wonder do you really need to check all
> those AVX512 flags in each case? Since "F" is always present in any AVX512
> implementation perhaps it can be checked, though if the other three always
> need to be checked I can understand if you want to keep it there for
> completeness. [Are all the other 3 used in your code?]
> 

As for me it is good to check all the flags supported by compiler. Some 
old (but still supported by dpdk) gcc can't compile the code in some 
circumstances. For example:

gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.12)   <-- pretty 
old but still supported, right?

gcc -march=native -dM -E - < /dev/null | grep "AVX512"
#define __AVX512F__ 1
#define __AVX512BW__ 1
#define __AVX512CD__ 1
#define __AVX512DQ__ 1

Does not support __AVX512VL__

from acl_run_avx512x8.h in first_trans8 there is 
_mm256_mmask_i32gather_epi32 which requires this flag, so compilation 
will fail.

> Acked-by: Bruce Richardson <bruce.richardson at intel.com>
> 

-- 
Regards,
Vladimir