RE: [PATCH v4] build: reduce use of AVX compiler flags

["Varghese, Vipin" <Vipin.Varghese@amd.com>]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Tuesday, June 10, 2025 8:37 PM
> To: Varghese, Vipin <Vipin.Varghese@amd.com>
> Cc: dev@dpdk.org; Song, Keesang <Keesang.Song@amd.com>
> Subject: Re: [PATCH v4] build: reduce use of AVX compiler flags
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Tue, Jun 10, 2025 at 02:52:24PM +0000, Varghese, Vipin wrote:
> > [Public]
> >
> > Snipped
> >
> > > >
> > > > In above log I get `2 instances of march`; logs `-march=native
> > > > -mrtm -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-
> format-
> > > truncation -Wno-address-of-packed-member -
> > > DRTE_LOG_DEFAULT_LOGTYPE=pmd.net.i40e -DCC_AVX512_SUPPORT
> -
> > > march=skylake-avx512`.
> > > >
> > > > Question-1: I think this is not expected right? The
> > > > `-march=native` is populated
> > > from `cflags` and `-march= skylake-avx512` is populated from `
> cc_avx512_flags`.
> > >
> > > The above command is correct. So long as the compiler supports
> > > AVX-512 we will always compile the AVX-512 code paths for runtime
> > > selection. In practice, all supported compilers have AVX-512
> > > support, so in reality we have the two scenarios you tested:
> > >
> > > * The target architecture e.g. znver3 in your case, doesn't support avx512,
> > >   so the meson.build file adds on the necessary flags to add this support,
> > >   i.e. that file is compiled with -march=skylake=avx512, which is the
> > >   minimum ISA that gives you the necessary support.
> > > * The target architecture, e.g. znver4, does support AVX-512, then no
> > >   additional flags are added and the files are compiled "as normal"
> > >
> > > In both these cases, whether the target architecture is specified as
> > > "native" or explicitly makes no difference.
> >
> > So, for files which needs avx512 support like acl_avx512, fib_tire_avx512 and
> others, we then only pass `cc_avx512_flags`.
> > All other cases this is ignored. Thank you that makes sense.
> >
> > >
> > > > Question-2: if the target is meet minimal ISA why not we use
> > > > `-march=x86-64-
> > > v4`?
> > > >
> > >
> > > Good point, that would indeed be better. I'm just not sure whether
> > > it is supported widely enough on our compilers. Do you know what gcc
> > > and clang versions support that target?
> >
> > As I recollect we have been using `x86-64-v4` this from gcc 12.3 and clang-14.
> > I am not sure if the older versions supports `avx512 that is
> > x86-64-v4`
> >
>
> Checking with godbolt.org, to see what versions give errors or not.
> * Clang accepts the argument from v12 onwards
> * GCC accepts the argument from v11 onwards.
>
> Unfortunately, that means that we need to at least keep some form of backward
> compatibility for older compilers until such time as we have those versions as
> minimum.
>
> If you are ok with the patch in general, I'll see if I can respin a version where we
> check for support for -march=x86-64-v4 and use that when we can, otherwise
> falling back to skylake-avx512 as here. Does that seem a reasonable approach?

Thank you Bruce, that will be great. First try for `x86-86-v4` if it fails fall back to `skylake-avx512`

>
> /Bruce