On Thu, Sep 8, 2016 at 5:24 PM, Petri Savolainen <[email protected] > wrote:
> Building ABI compatible or shared library are two different > targets. A shared library may be used also without ABI > compatibility. A new --enable-abi-compat configuration option > is introduced. By default libraries are not built in ABI compat > mode to enable function inlining. There is a noticeable > performance difference when e.g. odp_atomic_xxx calls > are not inlined. > Do we have quantification of this difference? I think the problem is that it's not clear what the use case is for shared-but-not-ABI-compatible builds since these cannot be part of any distro. If they're just being used privately, then why not just specify shared=no and get the full advantages of whatever inlining the implementation can offer? I have no problem with changing the control from today's shared=yes|no to something like profile=embedded|cloud if that would be more descriptive of the intent. With regard to the atomics, it may be possible to inline these and still be ABI compatible, in which case this could be added to the "cloud" (shared) profile standard, however careful testing would be needed and it would also mean that these routines could never be changed since any change would break compatibility. Prohibiting inlines of any kind remains the safest way to ensure that there are no implementation dependencies. Having shared=yes be the default seems to make sense since those interested in running in embedded environments for maximum performance would be expected to have no problem specifying additional options, while those using ODP on an initial/casual basis would enjoy the benefits of ABI compatibility at no extra effort. > > Signed-off-by: Petri Savolainen <[email protected]> > --- > configure.ac | 20 ++++++++----- > platform/linux-generic/.gitignore | 1 - > platform/linux-generic/include/odp/api/atomic.h | 2 +- > platform/linux-generic/include/odp/api/byteorder.h | 2 +- > .../linux-generic/include/odp/api/plat/inlines.h | 16 +++++++++++ > .../include/odp/api/plat/inlines.h.in | 33 > ---------------------- > platform/linux-generic/include/odp/api/std_clib.h | 2 +- > platform/linux-generic/include/odp/api/sync.h | 2 +- > platform/linux-generic/m4/configure.m4 | 3 +- > platform/linux-generic/odp_atomic.c | 2 +- > platform/linux-generic/odp_byteorder.c | 2 +- > platform/linux-generic/odp_std_clib.c | 2 +- > platform/linux-generic/odp_sync.c | 2 +- > 13 files changed, 38 insertions(+), 51 deletions(-) > delete mode 100644 platform/linux-generic/.gitignore > create mode 100644 platform/linux-generic/include/odp/api/plat/inlines.h > delete mode 100644 platform/linux-generic/include/odp/api/plat/inlines. > h.in > > diff --git a/configure.ac b/configure.ac > index 982aff7..583542f 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -176,13 +176,6 @@ AM_CONDITIONAL([test_example], [test x$test_example = > xyes ]) > AM_CONDITIONAL([HAVE_DOXYGEN], [test "x${DOXYGEN}" = "xdoxygen"]) > AM_CONDITIONAL([user_guide], [test "x${user_guides}" = "xyes" ]) > AM_CONDITIONAL([HAVE_MSCGEN], [test "x${MSCGEN}" = "xmscgen"]) > -if test x$enable_shared != xyes; > -then > - _ODP_INLINES="_ODP_INLINES" > -else > - _ODP_INLINES="_ODP_NO_INLINES" > -fi > -AC_SUBST(_ODP_INLINES) > > ############################################################ > ############## > # Setup doxygen documentation > @@ -225,6 +218,19 @@ AC_ARG_ENABLE([debug], > ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG=$ODP_DEBUG" > > ############################################################ > ############## > +# Enable/disable ABI compatible build > +########################################################### > ############### > +ODP_ABI_COMPAT=0 > +AC_ARG_ENABLE([abi-compat], > + [ --enable-abi-compat build in ABI specification compatible > mode], > + [if test "x$enableval" = "xyes"; then > + ODP_ABI_COMPAT=1 > + else > + ODP_ABI_COMPAT=0 > + fi]) > +ODP_CFLAGS="$ODP_CFLAGS -DODP_ABI_COMPAT=$ODP_ABI_COMPAT" > + > +########################################################### > ############### > # Default warning setup > ############################################################ > ############## > ODP_CFLAGS="$ODP_CFLAGS -W -Wall -Werror -Wstrict-prototypes > -Wmissing-prototypes" > diff --git a/platform/linux-generic/.gitignore b/platform/linux-generic/. > gitignore > deleted file mode 100644 > index ec6ca37..0000000 > --- a/platform/linux-generic/.gitignore > +++ /dev/null > @@ -1 +0,0 @@ > -include/odp/api/plat/inlines.h > diff --git a/platform/linux-generic/include/odp/api/atomic.h > b/platform/linux-generic/include/odp/api/atomic.h > index c18e68b..66cfef7 100644 > --- a/platform/linux-generic/include/odp/api/atomic.h > +++ b/platform/linux-generic/include/odp/api/atomic.h > @@ -25,7 +25,7 @@ extern "C" { > */ > > #include <odp/api/plat/inlines.h> > -#ifdef _ODP_INLINES > +#if ODP_ABI_COMPAT == 0 > #include <odp/api/plat/atomic_inlines.h> > #endif > > diff --git a/platform/linux-generic/include/odp/api/byteorder.h > b/platform/linux-generic/include/odp/api/byteorder.h > index 84d1173..37f5636 100644 > --- a/platform/linux-generic/include/odp/api/byteorder.h > +++ b/platform/linux-generic/include/odp/api/byteorder.h > @@ -26,7 +26,7 @@ extern "C" { > */ > > #include <odp/api/plat/inlines.h> > -#ifdef _ODP_INLINES > +#if ODP_ABI_COMPAT == 0 > #include <odp/api/plat/byteorder_inlines.h> > #endif > > diff --git a/platform/linux-generic/include/odp/api/plat/inlines.h > b/platform/linux-generic/include/odp/api/plat/inlines.h > new file mode 100644 > index 0000000..e042bd7 > --- /dev/null > +++ b/platform/linux-generic/include/odp/api/plat/inlines.h > @@ -0,0 +1,16 @@ > +/* Copyright (c) 2016, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +/** > + * @file > + * > + * Macro for static inline functions > + */ > +#if ODP_ABI_COMPAT == 0 > +#define _STATIC static inline > +#else > +#define _STATIC > +#endif > diff --git a/platform/linux-generic/include/odp/api/plat/inlines.h.in > b/platform/linux-generic/include/odp/api/plat/inlines.h.in > deleted file mode 100644 > index 5d8c0dc..0000000 > --- a/platform/linux-generic/include/odp/api/plat/inlines.h.in > +++ /dev/null > @@ -1,33 +0,0 @@ > -/* Copyright (c) 2016, Linaro Limited > - * All rights reserved. > - * > - * SPDX-License-Identifier: BSD-3-Clause > - */ > - > -/** > - * @file > - * > - * ODP platform inline functions > - */ > - > -#ifndef ODP_PLAT_INLINES_H_ > -#define ODP_PLAT_INLINES_H_ > - > -#ifdef __cplusplus > -extern "C" { > -#endif > - > -#define @_ODP_INLINES@ > - > -#ifdef _ODP_INLINES > -#define _STATIC static inline > -#else > -#define _STATIC > -#endif > - > -#ifdef __cplusplus > -} > -#endif > - > - > -#endif /* ODP_PLAT_INLINES_H_ */ > diff --git a/platform/linux-generic/include/odp/api/std_clib.h > b/platform/linux-generic/include/odp/api/std_clib.h > index c498f68..7fcd6f3 100644 > --- a/platform/linux-generic/include/odp/api/std_clib.h > +++ b/platform/linux-generic/include/odp/api/std_clib.h > @@ -15,7 +15,7 @@ extern "C" { > #include <string.h> > > #include <odp/api/plat/inlines.h> > -#ifdef _ODP_INLINES > +#if ODP_ABI_COMPAT == 0 > #include <odp/api/plat/std_clib_inlines.h> > #endif > > diff --git a/platform/linux-generic/include/odp/api/sync.h > b/platform/linux-generic/include/odp/api/sync.h > index d2becb9..1423a69 100644 > --- a/platform/linux-generic/include/odp/api/sync.h > +++ b/platform/linux-generic/include/odp/api/sync.h > @@ -22,7 +22,7 @@ extern "C" { > */ > > #include <odp/api/plat/inlines.h> > -#ifdef _ODP_INLINES > +#if ODP_ABI_COMPAT == 0 > #include <odp/api/plat/sync_inlines.h> > #endif > > diff --git a/platform/linux-generic/m4/configure.m4 > b/platform/linux-generic/m4/configure.m4 > index 6fb05c0..1b1b883 100644 > --- a/platform/linux-generic/m4/configure.m4 > +++ b/platform/linux-generic/m4/configure.m4 > @@ -36,5 +36,4 @@ m4_include([platform/linux-generic/m4/odp_dpdk.m4]) > m4_include([platform/linux-generic/m4/odp_ipc.m4]) > m4_include([platform/linux-generic/m4/odp_schedule.m4]) > > -AC_CONFIG_FILES([platform/linux-generic/Makefile > - platform/linux-generic/include/odp/api/plat/inlines.h]) > +AC_CONFIG_FILES([platform/linux-generic/Makefile]) > diff --git a/platform/linux-generic/odp_atomic.c > b/platform/linux-generic/odp_atomic.c > index e9a3ed0..0e40cda 100644 > --- a/platform/linux-generic/odp_atomic.c > +++ b/platform/linux-generic/odp_atomic.c > @@ -5,7 +5,7 @@ > */ > > #include <odp/api/atomic.h> > -#ifndef _ODP_INLINES > +#if ODP_ABI_COMPAT == 1 > #include <odp/api/plat/atomic_inlines.h> > #endif > > diff --git a/platform/linux-generic/odp_byteorder.c > b/platform/linux-generic/odp_byteorder.c > index fc87291..a344c53 100644 > --- a/platform/linux-generic/odp_byteorder.c > +++ b/platform/linux-generic/odp_byteorder.c > @@ -5,6 +5,6 @@ > */ > > #include <odp/api/byteorder.h> > -#ifndef _ODP_INLINES > +#if ODP_ABI_COMPAT == 1 > #include <odp/api/plat/byteorder_inlines.h> > #endif > diff --git a/platform/linux-generic/odp_std_clib.c > b/platform/linux-generic/odp_std_clib.c > index 611ba12..24df249 100644 > --- a/platform/linux-generic/odp_std_clib.c > +++ b/platform/linux-generic/odp_std_clib.c > @@ -5,6 +5,6 @@ > */ > > #include <odp/api/std_clib.h> > -#ifndef _ODP_INLINES > +#if ODP_ABI_COMPAT == 1 > #include <odp/api/plat/std_clib_inlines.h> > #endif > diff --git a/platform/linux-generic/odp_sync.c > b/platform/linux-generic/odp_sync.c > index f31c389..b7eb503 100644 > --- a/platform/linux-generic/odp_sync.c > +++ b/platform/linux-generic/odp_sync.c > @@ -5,6 +5,6 @@ > */ > > #include <odp/api/sync.h> > -#ifndef _ODP_INLINES > +#if ODP_ABI_COMPAT == 1 > #include <odp/api/plat/sync_inlines.h> > #endif > -- > 2.8.1 > >
