On Fri, Jul 18, 2025 at 3:06 PM Bruce Richardson <bruce.richard...@intel.com> wrote: > > On Fri, Jul 18, 2025 at 02:45:48PM +0200, David Marchand wrote: > > On Fri, Jul 18, 2025 at 2:14 PM Bruce Richardson > > <bruce.richard...@intel.com> wrote: > > > diff --git a/drivers/net/virtio/meson.build > > > b/drivers/net/virtio/meson.build > > > index d3caa3a3b4..a9ff87e863 100644 > > > --- a/drivers/net/virtio/meson.build > > > +++ b/drivers/net/virtio/meson.build > > > @@ -31,7 +31,7 @@ if arch_subdir == 'x86' > > > sources_avx512 += files('virtio_rxtx_packed.c') > > > if (toolchain == 'gcc' and > > > cc.version().version_compare('>=8.3.0')) > > > cflags += '-DVIRTIO_GCC_UNROLL_PRAGMA' > > > - elif (toolchain == 'clang' and > > > cc.version().version_compare('>=3.7.0')) > > > + elif (toolchain == 'clang') > > > cflags += '-DVIRTIO_CLANG_UNROLL_PRAGMA' > > > endif > > > endif > > > > One other nit, not blocking. > > > > There should be no need for special casing clang vs gcc, since clang > > supports gcc syntax in general. > > https://clang.llvm.org/docs/AttributeReference.html#pragma-unroll-pragma-nounroll > > > I was actually thinking of doing a follow-up patch to remove the "if" and > instead do: > > cflags += '-DVIRTIO_' + toolchain.to_upper() + "_UNROLL_PRAGMA" > > on the basis that having a define for any other unknown compilers would be > harmless. However, you are right that there seems to be little reason to do > this in meson.build, and the code can do it directly itself. I'll leave > this part as it is in the patch for now.
Cc: Maxime (who is off atm) I wonder if this pragma stuff really helps.. One quick test on the loop in vhost_flush_enqueue_batch_packed() shows the same generated code with gcc 10 and gcc 15 when compiling with -O3 (thanks to godbolt.org). Let's leave it as is for now, but I would be for dropping this strange construct, only used with the virtio ring packed layout iirc. -- David Marchand