Re: [PATCH] build: remove obsolete compiler checks

[David Marchand <david.marchand@redhat.com>]

On Fri, Jul 18, 2025 at 3:06 PM Bruce Richardson
<bruce.richardson@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.richardson@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