On Tue, Oct 08, 2024 at 01:33:16PM +0200, David Marchand wrote: > 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? > Yep.
> > 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? > I'll see what I can do today. /Bruce
