On Tue, Oct 8, 2024 at 12:03 PM Bruce Richardson <bruce.richard...@intel.com> wrote: > > On Tue, Oct 08, 2024 at 10:49:39AM +0200, David Marchand wrote: > > On Tue, Oct 1, 2024 at 1:19 PM Bruce Richardson > > <bruce.richard...@intel.com> wrote: > > > > > > The meson code to detect CPU and compiler support for AVX512 was > > > duplicated > > > across multiple drivers. Do all detection in just a single place to > > > simplify > > > the code. > > > > > > v2: ensure that target_has_avx512 is always defined on x86 to fix build > > > errors > > > > > > Bruce Richardson (8): > > > config/x86: add global defines for checking AVX-512 > > > event/dlb2: use global AVX-512 variables > > > common/idpf: use global AVX-512 variables > > > net/cpfl: use global AVX-512 variables > > > net/i40e: use global AVX-512 variables > > > net/iavf: use global AVX-512 variables > > > net/ice: use global AVX-512 variables > > > net/idpf: use global AVX-512 variables > > > > > > config/x86/meson.build | 19 +++++++++++---- > > > drivers/common/idpf/meson.build | 17 ++----------- > > > drivers/event/dlb2/meson.build | 42 +++++++-------------------------- > > > drivers/net/cpfl/meson.build | 19 ++------------- > > > drivers/net/i40e/meson.build | 13 ++-------- > > > drivers/net/iavf/meson.build | 13 ++-------- > > > drivers/net/ice/meson.build | 15 ++---------- > > > drivers/net/idpf/meson.build | 19 ++------------- > > > 8 files changed, 36 insertions(+), 121 deletions(-) > > > > Thanks for this cleanup, I have two comments. > > > > - Some drivers were going into great lenghts to check that individiual > > avx512 features were available. > > With this series, we end up requiring support for all features to > > announce avx512 availability. > > Are we perhaps disabling AVX512 support with some toolchains, out > > there, supporting only part of the set? > > > > The various AVX-512 feature sets checked for (F, BW, VL, DQ) were all > introduced in the same hardware generation - all are available in gcc when > using -march=skylake-avx512 or later, or -march=znver4. On the toolchain > side, gcc introduced all these flags simultaneously in gcc-6 [1]. For > clang/llvm, testing with godbolt for compiler errors/warnings indicates > that all these 4 avx512 flags are available from clang 3.6 - the minimum we > support in DPDK [2] > > [1] https://gcc.gnu.org/gcc-6/changes.html > [2] > https://doc.dpdk.org/guides/linux_gsg/sys_reqs.html#compilation-of-the-dpdk
Perfect, thanks for the details. > > > - Some drivers were checking for presence of -mno-avx512f in > > machine_args as a way to disable building any AVX512 stuff. > > This gets discarded with this series. > > > > Yes, because it should no longer be necessary. The places in the build > system where we set the no-avx512f flag are reworked so that we don't have > cc_has_avx512 set. Ok, it is clearer now. Last comment on style: $ git grep cc_avx512_flags drivers/ drivers/common/idpf/meson.build: avx512_args = [cflags] + cc_avx512_flags drivers/event/dlb2/meson.build: c_args: cflags + cc_avx512_flags) drivers/net/i40e/meson.build: avx512_args = cflags + cc_avx512_flags drivers/net/iavf/meson.build: avx512_args = cflags + cc_avx512_flags drivers/net/ice/meson.build: avx512_args = cflags + cc_avx512_flags I think it is safe to remove the [] around cflags in common/idpf, right? Do you have some cycles to send a v2 and convert lib/net and net/virtio ? Otherwise, can you do a followup patch for rc2? Thanks. -- David Marchand
