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

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

 
> 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.ananyev@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.