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
