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?
/Bruce
