> On Wed, Sep 16, 2020 at 10:36:32AM +0100, Medvedkin, Vladimir wrote: > > 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?]
Yep, ACL uses all of them. Thanks Konstantin > > > > > > > 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__ > > > Interesting, seems like checking them all to be sure is the right approach > so. > My ack stands so, and ignore the comment.
