[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Bruce Richardson <bruce.richard...@intel.com>
> Sent: Monday, June 9, 2025 1:28 PM
> To: Varghese, Vipin <vipin.vargh...@amd.com>
> Cc: dev@dpdk.org; Song, Keesang <keesang.s...@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 Mon, Jun 09, 2025 at 06:02:02AM +0000, Varghese, Vipin wrote:
> > [Public]
> >
> > Snipped
> >
> > >
> > >
> > > When doing a build for a target that already has the instruction
> > > sets for
> > > AVX2/AVX512 enabled, skip emitting the AVX compiler flags, or the
> > > skylake-avx512 '-march' flags, as they are unnecessary. Instead,
> > > when the default flags produce the desired output, just use them
> > > unmodified, and don't bother adding in extra enabling flags for AVX2 or 
> > > AVX-512.
> > >
> > > Depends-on: series-35006 ("doc/linux_gsg: update recommended
> > > compiler
> > > versions")
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richard...@intel.com>
> > > ---
> > >
> > > V4: Fix error flagged by CI with clang builds without AVX512 - change
> > >     "cc_avx512_args" to correct "cc_avx512_flags"
> > >
> > > V3: put in version check to work around an issues with some meson
> > >     versions, (hopefully) allowing builds to pass in all CIs. The
> > >     printout of the extra flags now only happens with meson >=
> > > 0.60.2
> > >
> > > V2: dropped the doc update for the minimum compiler version.  Based on
> > >     discussion, that version bump is larger than proposed in RFC and is
> > >     now a separate patch/series [series 35006 referenced above]
> > >
> > > ---
> > >  config/x86/meson.build | 31 ++++++++++++++++++++-----------
> > >  drivers/meson.build    |  9 +--------
> > >  lib/meson.build        |  9 +--------
> > >  3 files changed, 22 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/config/x86/meson.build b/config/x86/meson.build index
> > > c3564b0011..e6612dbd80 100644
> > > --- a/config/x86/meson.build
> > > +++ b/config/x86/meson.build
> > > @@ -4,11 +4,13 @@
> > >  if is_ms_compiler
> > >      cc_avx2_flags = ['/arch:AVX2']
> > >  else
> > > -    cc_avx2_flags = ['-mavx2']
> > > +    cc_avx2_flags = []
> > > +    if cc.get_define('__AVX2__', args: machine_args) == ''
> > > +        cc_avx2_flags = ['-mavx2']
> > > +    endif
> > >  endif
> > >
> > >  cc_has_avx512 = false
> > > -target_has_avx512 = false
> > >
> > >  dpdk_conf.set('RTE_ARCH_X86', 1)
> > >  if dpdk_conf.get('RTE_ARCH_64')
> > > @@ -65,26 +67,33 @@ if is_linux or cc.get_id() == 'gcc'
> > >      endif
> > >  endif
> > >
> > > -cc_avx512_flags = ['-mavx512f', '-mavx512vl', '-mavx512dq',
> > > '-mavx512bw', '- mavx512cd'] -if (binutils_ok and
> > > cc.has_multi_arguments(cc_avx512_flags)
> > > +avx512_march_flag = '-march=skylake-avx512'
> > > +cc_avx512_flags = []
> > > +if (binutils_ok and cc.has_argument(avx512_march_flag)
> > >          and '-mno-avx512f' not in get_option('c_args'))
> > >      # check if compiler is working with _mm512_extracti64x4_epi64
> > >      # Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82887
> > >      code = '''#include <immintrin.h>
> > >      void test(__m512i zmm){
> > >          __m256i ymm = _mm512_extracti64x4_epi64(zmm, 0);}'''
> > > -    result = cc.compiles(code, args : cc_avx512_flags, name : 'AVX512
> checking')
> > > +    result = cc.compiles(code, args : [avx512_march_flag], name :
> > > + 'AVX512 checking')
> > >      if result == false
> > >          machine_args += '-mno-avx512f'
> > >          warning('Broken _mm512_extracti64x4_epi64, disabling AVX512 
> > > support')
> > >      else
> > >          cc_has_avx512 = true
> > > -        target_has_avx512 = (
> > > -                cc.get_define('__AVX512F__', args: machine_args) != '' 
> > > and
> > > -                cc.get_define('__AVX512BW__', args: machine_args) != '' 
> > > and
> > > -                cc.get_define('__AVX512DQ__', args: machine_args) != '' 
> > > and
> > > -                cc.get_define('__AVX512VL__', args: machine_args) != ''
> > > -            )
> > > +        if cc.get_define('__AVX512F__', args: machine_args) == ''
> > > +            cc_avx512_flags = [avx512_march_flag]
> >
> > Hi Bruce, we have reviewed this internally and tested the same. We would 
> > like
> your thought for the following.
> >
> > - Before patch: we were directly setting AVX512 falgs for F, BW, DQ,
> > VL
> > - new patch: we are setting the flags for `skylake-server` as bare minimal.
> > - AMD supports AVX512 from `znver4 and higher`.
> >
> > As per GCC `https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html`, the extra 
> > ISA
> supported between skylake-server (super set) and znver4 and znver5 are `SAHF,
> FXSR, XSAVE, RDRND, LZCNT, HLE, PREFETCHW, SGX`.
> > Currently for DPDK microbenchmarks and examples runs safe as it is not using
> the `SAHF, FXSR, XSAVE, RDRND, LZCNT, HLE, PREFETCHW, SGX`
> instructions.
> >
> > Question: should we check if target is `AMD EPYC` then apply bare minimum as
> `-march=znver4`, thus avoid possible unsupported instruction generation when 
> non
> `c_args for march` is passed?
> >
>
> Can you clarify why you mean by the "target" here? Is there a specific value 
> you
> are thinking of for the "cpu_instruction_set" option?

`Target` is target CPU, when generated without any arguments we get code for 
`native build`.

On AMD target cpu zen4 or zen5; Before patch as per the code ` AVX512 flags for 
F, BW, DQ` are used in ` cc_avx512_flags`.

With the patch, the cc_avx512_flags is set to `-march=skylake-avx512` (where 
compiler optimizations `can add HLE, PREFETCHW, SGX`).

Note: in our current testing for dpdk app and a few examples, we have not 
encountered the same yet. But there might be projects where compiler may add 
those instructions which are unsupported in `AMD EPYC`.

Hence the question we are asking is, `should we check if target is AMD-EPYC, if 
yes use -march=znver4 instead of -march=skylake-server`?

>
> /Bruce

Reply via email to