[v4,02/14] vhost: unify unroll pragma parameter
Checks
Commit Message
Add macro for unifying Clang/ICC/GCC unroll pragma format. Batch
functions were contained of several small loops which optimized by
compiler’s loop unrolling pragma.
Signed-off-by: Marvin Liu <yong.liu@intel.com>
Comments
On 10/9/19 3:38 PM, Marvin Liu wrote:
> Add macro for unifying Clang/ICC/GCC unroll pragma format. Batch
> functions were contained of several small loops which optimized by
> compiler’s loop unrolling pragma.
>
> Signed-off-by: Marvin Liu <yong.liu@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)
> +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
> +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/meson.build b/lib/librte_vhost/meson.build
> index cb1123ae3..ddf0ee579 100644
> --- a/lib/librte_vhost/meson.build
> +++ b/lib/librte_vhost/meson.build
> @@ -8,6 +8,13 @@ endif
> if has_libnuma == 1
> dpdk_conf.set10('RTE_LIBRTE_VHOST_NUMA', true)
> endif
> +if (toolchain == 'gcc' and cc.version().version_compare('>=8.3.0'))
> + cflags += '-DSUPPORT_GCC_UNROLL_PRAGMA'
> +elif (toolchain == 'clang' and cc.version().version_compare('>=3.7.0'))
> + cflags += '-DSUPPORT_CLANG_UNROLL_PRAGMA'
> +elif (toolchain == 'icc' and cc.version().version_compare('>=16.0.0'))
> + cflags += '-DSUPPORT_ICC_UNROLL_PRAGMA'
> +endif
> dpdk_conf.set('RTE_LIBRTE_VHOST_POSTCOPY',
> cc.has_header('linux/userfaultfd.h'))
> version = 4
> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
> index 884befa85..4cba8c5ef 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 UNROLL_PRAGMA_PARAM "GCC unroll 4"
Shouldn't al these defines be either prefixed with VHOST_, or being
declared in EAL headers, so that it can be used by other DPDK libs?
I will pick it as is for now, but please consider above comment and
and send a patch on top if it makes sense.
Thanks,
Maxime
> +#endif
> +
> +#ifdef SUPPORT_CLANG_UNROLL_PRAGMA
> +#define UNROLL_PRAGMA_PARAM "unroll 4"
> +#endif
> +
> +#ifdef SUPPORT_ICC_UNROLL_PRAGMA
> +#define UNROLL_PRAGMA_PARAM "unroll (4)"
> +#endif
> +
> +#ifdef UNROLL_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.
>
> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Friday, October 11, 2019 8:49 PM
> To: Liu, Yong <yong.liu@intel.com>; Bie, Tiwei <tiwei.bie@intel.com>; Wang,
> Zhihong <zhihong.wang@intel.com>; stephen@networkplumber.org;
> gavin.hu@arm.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v4 02/14] vhost: unify unroll pragma parameter
>
>
>
> On 10/9/19 3:38 PM, Marvin Liu wrote:
> > Add macro for unifying Clang/ICC/GCC unroll pragma format. Batch
> > functions were contained of several small loops which optimized by
> > compiler’s loop unrolling pragma.
> >
> > Signed-off-by: Marvin Liu <yong.liu@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)
> > +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
> > +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/meson.build b/lib/librte_vhost/meson.build
> > index cb1123ae3..ddf0ee579 100644
> > --- a/lib/librte_vhost/meson.build
> > +++ b/lib/librte_vhost/meson.build
> > @@ -8,6 +8,13 @@ endif
> > if has_libnuma == 1
> > dpdk_conf.set10('RTE_LIBRTE_VHOST_NUMA', true)
> > endif
> > +if (toolchain == 'gcc' and cc.version().version_compare('>=8.3.0'))
> > + cflags += '-DSUPPORT_GCC_UNROLL_PRAGMA'
> > +elif (toolchain == 'clang' and cc.version().version_compare('>=3.7.0'))
> > + cflags += '-DSUPPORT_CLANG_UNROLL_PRAGMA'
> > +elif (toolchain == 'icc' and cc.version().version_compare('>=16.0.0'))
> > + cflags += '-DSUPPORT_ICC_UNROLL_PRAGMA'
> > +endif
> > dpdk_conf.set('RTE_LIBRTE_VHOST_POSTCOPY',
> > cc.has_header('linux/userfaultfd.h'))
> > version = 4
> > diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
> > index 884befa85..4cba8c5ef 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 UNROLL_PRAGMA_PARAM "GCC unroll 4"
>
> Shouldn't al these defines be either prefixed with VHOST_, or being
> declared in EAL headers, so that it can be used by other DPDK libs?
>
> I will pick it as is for now, but please consider above comment and
> and send a patch on top if it makes sense.
>
Hi Maxime,
For making loop unroll macro more generic, modified version as below.
Since only vhost utilize the benefit of compiler's unroll feature, I'd like to keep it in vhost by now.
#ifdef SUPPORT_GCC_UNROLL_PRAGMA
#define for_each_try_unroll(iter, val, size) _Pragma("GCC unroll 4") \
for (iter = val; iter < size; iter++)
#endif
#ifdef SUPPORT_CLANG_UNROLL_PRAGMA
#define for_each_try_unroll(iter, val, size) _Pragma("unroll 4") \
for (iter = val; iter < size; iter++)
#endif
#ifdef SUPPORT_ICC_UNROLL_PRAGMA
#define for_each_try_unroll(iter, val, size) _Pragma("unroll (4)") \
for (iter = val; iter < size; iter++)
#endif
#ifndef for_each_try_unroll
#define for_each_try_unroll(iter, val, num) \
for (iter = val; iter < num; iter++)
#endif
Regards,
Marvin
> Thanks,
> Maxime
> > +#endif
> > +
> > +#ifdef SUPPORT_CLANG_UNROLL_PRAGMA
> > +#define UNROLL_PRAGMA_PARAM "unroll 4"
> > +#endif
> > +
> > +#ifdef SUPPORT_ICC_UNROLL_PRAGMA
> > +#define UNROLL_PRAGMA_PARAM "unroll (4)"
> > +#endif
> > +
> > +#ifdef UNROLL_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.
> >
@@ -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)
+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
+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
@@ -8,6 +8,13 @@ endif
if has_libnuma == 1
dpdk_conf.set10('RTE_LIBRTE_VHOST_NUMA', true)
endif
+if (toolchain == 'gcc' and cc.version().version_compare('>=8.3.0'))
+ cflags += '-DSUPPORT_GCC_UNROLL_PRAGMA'
+elif (toolchain == 'clang' and cc.version().version_compare('>=3.7.0'))
+ cflags += '-DSUPPORT_CLANG_UNROLL_PRAGMA'
+elif (toolchain == 'icc' and cc.version().version_compare('>=16.0.0'))
+ cflags += '-DSUPPORT_ICC_UNROLL_PRAGMA'
+endif
dpdk_conf.set('RTE_LIBRTE_VHOST_POSTCOPY',
cc.has_header('linux/userfaultfd.h'))
version = 4
@@ -39,6 +39,24 @@
#define VHOST_LOG_CACHE_NR 32
+#ifdef SUPPORT_GCC_UNROLL_PRAGMA
+#define UNROLL_PRAGMA_PARAM "GCC unroll 4"
+#endif
+
+#ifdef SUPPORT_CLANG_UNROLL_PRAGMA
+#define UNROLL_PRAGMA_PARAM "unroll 4"
+#endif
+
+#ifdef SUPPORT_ICC_UNROLL_PRAGMA
+#define UNROLL_PRAGMA_PARAM "unroll (4)"
+#endif
+
+#ifdef UNROLL_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.