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.anan...@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.richard...@intel.com>


--
Regards,
Vladimir

Reply via email to