> -----Original Message-----
> From: Gavin Hu (Arm Technology China) [mailto:gavin...@arm.com]
> Sent: Monday, September 23, 2019 6:09 PM
> To: Liu, Yong <yong....@intel.com>; maxime.coque...@redhat.com; Bie, Tiwei
> <tiwei....@intel.com>; Wang, Zhihong <zhihong.w...@intel.com>; Richardson,
> Bruce <bruce.richard...@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 02/16] vhost: unify unroll pragma
> parameter
>
> Hi Marvin,
>
> One general comment and other comments inline:
> 1. Meson build should also be supported as Makefile is phasing out and
> Meson is the future in DPDK.
>
> /Gavin
>
Thanks, Gavin. Will update meson build file in next release.
> > -----Original Message-----
> > From: dev <dev-boun...@dpdk.org> On Behalf Of Marvin Liu
> > Sent: Friday, September 20, 2019 12:36 AM
> > To: maxime.coque...@redhat.com; tiwei....@intel.com;
> > zhihong.w...@intel.com
> > Cc: dev@dpdk.org; Marvin Liu <yong....@intel.com>
> > Subject: [dpdk-dev] [PATCH v2 02/16] vhost: unify unroll pragma parameter
> >
> > Add macro for unifying Clang/ICC/GCC unroll pragma format. Burst
> > functions were contained of several small loops which optimized by
> > compiler’s loop unrolling pragma.
> >
> > Signed-off-by: Marvin Liu <yong....@intel.com>
> >
> > diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile
> > index 8623e91c0..30839a001 100644
> > --- a/lib/librte_vhost/Makefile
> > +++ b/lib/librte_vhost/Makefile
> > @@ -16,6 +16,24 @@ CFLAGS += -I vhost_user
> > CFLAGS += -fno-strict-aliasing
> > LDLIBS += -lpthread
> >
> > +ifeq ($(RTE_TOOLCHAIN), gcc)
> > +ifeq ($(shell test $(GCC_VERSION) -ge 83 && echo 1), 1)
> It is better to move this toolchain version related definition to eg:
> mk/toolchain/icc/rte.toolchain-compat.mk.
> There are a lot of similar stuff over there.
> Although "CFLAGS" was added to sth under this subfolder, it still applies
> globally to other components.
> /Gavin
> > +CFLAGS += -DSUPPORT_GCC_UNROLL_PRAGMA
> > +endif
> > +endif
> > +
> > +ifeq ($(RTE_TOOLCHAIN), clang)
> > +ifeq ($(shell test $(CLANG_MAJOR_VERSION)$(CLANG_MINOR_VERSION) -
> > ge 37 && echo 1), 1)
> > +CFLAGS += -DSUPPORT_CLANG_UNROLL_PRAGMA
> Why not combine all the three "-DSUPPORT_*_UNROLL_PRAGMA" into one "-
> DSUPPORT_ UNROLL_PRAGMA" for simplicity?
> Any differences for the support by different compilers?
> /Gavin
Gavin,
This is due to parameter format of pragmas are different between compilers. So
here created several macros for each compiler.
> > +endif
> > +endif
> > +
> > +ifeq ($(RTE_TOOLCHAIN), icc)
> > +ifeq ($(shell test $(ICC_MAJOR_VERSION) -ge 16 && echo 1), 1)
> > +CFLAGS += -DSUPPORT_ICC_UNROLL_PRAGMA
> > +endif
> > +endif
> > +
> > ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
> > LDLIBS += -lnuma
> > endif
> > diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
> > index 884befa85..5074226f0 100644
> > --- a/lib/librte_vhost/vhost.h
> > +++ b/lib/librte_vhost/vhost.h
> > @@ -39,6 +39,24 @@
> >
> > #define VHOST_LOG_CACHE_NR 32
> >
> > +#ifdef SUPPORT_GCC_UNROLL_PRAGMA
> > +#define PRAGMA_PARAM "GCC unroll 4"
> > +#endif
> > +
> > +#ifdef SUPPORT_CLANG_UNROLL_PRAGMA
> > +#define PRAGMA_PARAM "unroll 4"
> > +#endif
> > +
> > +#ifdef SUPPORT_ICC_UNROLL_PRAGMA
> > +#define PRAGMA_PARAM "unroll (4)"
> > +#endif
> > +
> > +#ifdef PRAGMA_PARAM
> > +#define UNROLL_PRAGMA(param) _Pragma(param)
> > +#else
> > +#define UNROLL_PRAGMA(param) do {} while(0);
> > +#endif
> > +
> > /**
> > * Structure contains buffer address, length and descriptor index
> > * from vring to do scatter RX.
> > --
> > 2.17.1
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
