On Tue, Aug 19, 2025 at 10:10:21AM +0200, David Marchand wrote:
> Hello Bruce,
>
> On Fri, Jul 25, 2025 at 6:07 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
>
> [snip]
>
> > diff --git a/lib/vhost/meson.build b/lib/vhost/meson.build
> > index 9c6325aa0e..a74d0cff7c 100644
> > --- a/lib/vhost/meson.build
> > +++ b/lib/vhost/meson.build
> > @@ -8,9 +8,9 @@ endif
> > if has_libnuma
> > dpdk_conf.set10('RTE_LIBRTE_VHOST_NUMA', true)
> > endif
> > -if (toolchain == 'gcc' and cc.version().version_compare('>=8.3.0'))
> > +if (toolchain == 'gcc')
> > cflags += '-DVHOST_GCC_UNROLL_PRAGMA'
> > -elif (toolchain == 'clang' and cc.version().version_compare('>=3.7.0'))
> > +elif (toolchain == 'clang')
> > cflags += '-DVHOST_CLANG_UNROLL_PRAGMA'
> > endif
> > dpdk_conf.set('RTE_LIBRTE_VHOST_POSTCOPY',
> > cc.has_header('linux/userfaultfd.h'))
>
> Sorry, I did not notice in the v1, but lib/vhost and
> drivers/net/virtio have the same construct, yet only lib/vhost gets
> updated on the gcc version check in this patch.
> Is this intentional?
Not that I remember, I seem to have missed that.
>
> Since GCC 8.0+ is recommended, I would keep the version check in both
> vhost and net/virtio.
I think I actually checked this out, and decided that it was ok to drop the
check. I will reconfirm that, see how this is used and if it really
needs 8.3 GCC.
>
> Other than this, it lgtm.
> Reviewed-by: David Marchand <david.march...@redhat.com>
>
>
> --
> David Marchand
>
