Actually, I have to withdraw my previous review of this patch. The problem
is that the original works as part of autoconf to create the plat/inlines.h
file from an inlines.h.in base while this one attempts to do this by
specifying CFLAGs that #define ODP_ABI_COMPAT.
That works fine if you're compiling from the odp git repo but fails if
you've installed a copy of ODP elsewhere. To see this:
./bootstrap
./configure --prefix=<odp-install-path> ...any other options
make
make install
Then try to build an external ODP application using this installed copy.
I've used OFP here for convenience.
./bootstrap
./configure --with-odp=<odp-install-path> ...any other options
make
and you get lots of error messages of the form:
CC cli/ofp_cli_alias.lo
In file included from /tmp/odp/include/odp/api/byteorder.h:28:0,
from /tmp/odp/include/odp_api.h:28,
from /tmp/odp/include/odp.h:21,
from ../include/api/ofp_log.h:17,
from ../include/ofpi_log.h:8,
from cli/ofp_cli_alias.c:12:
/tmp/odp/include/odp/api/plat/inlines.h:20:5: warning: "ODP_ABI_COMPAT" is
not defined [-Wundef]
#if ODP_ABI_COMPAT == 0
^
In file included from /tmp/odp/include/odp_api.h:28:0,
from /tmp/odp/include/odp.h:21,
from ../include/api/ofp_log.h:17,
from ../include/ofpi_log.h:8,
from cli/ofp_cli_alias.c:12:
/tmp/odp/include/odp/api/byteorder.h:29:5: warning: "ODP_ABI_COMPAT" is not
defined [-Wundef]
#if ODP_ABI_COMPAT == 0
^
In file included from /tmp/odp/include/odp/api/barrier.h:21:0,
from /tmp/odp/include/odp_api.h:31,
from /tmp/odp/include/odp.h:21,
from ../include/api/ofp_log.h:17,
from ../include/ofpi_log.h:8,
from cli/ofp_cli_alias.c:12:
/tmp/odp/include/odp/api/atomic.h:28:5: warning: "ODP_ABI_COMPAT" is not
defined [-Wundef]
#if ODP_ABI_COMPAT == 0
^
...etc.
This is because the CFLAGS that were set to compile ODP aren't present when
an application using the installed ODP library is subsequently compiled
against the library. The installed library itself must contain the
different expansion depending on whether --enable-abi-compat=[yes|no] was
specified, and that means this needs to be done via autoconf, not a #define.
On Wed, Sep 14, 2016 at 7:10 AM, Petri Savolainen <
petri.savolai...@nokia.com> 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.
>
> Signed-off-by: Petri Savolainen <petri.savolai...@nokia.com>
> ---
>
> v2:
> * ABI compat enabled by default
> * print static/shared/abi_compat selection in config results
> * added missing header file include guards
>
>
> configure.ac | 26 ++++++++++++-----
> 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 | 30
> ++++++++++++++++++++
> .../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, 58 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..e05d4dd 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,22 @@ AC_ARG_ENABLE([debug],
> ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG=$ODP_DEBUG"
>
> ############################################################
> ##############
> +# Enable/disable ABI compatible build
> +###########################################################
> ###############
> +ODP_ABI_COMPAT=1
> +abi_compat=yes
> +AC_ARG_ENABLE([abi-compat],
> + [ --enable-abi-compat build all targets in ABI compatible mode
> (default=yes)],
> + [if test "x$enableval" = "xyes"; then
> + ODP_ABI_COMPAT=1
> + abi_compat=yes
> + else
> + ODP_ABI_COMPAT=0
> + abi_compat=no
> + 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"
> @@ -307,6 +316,9 @@ AC_MSG_RESULT([
> am_ldflags: ${AM_LDFLAGS}
> libs: ${LIBS}
> defs: ${DEFS}
> + static libraries: ${enable_static}
> + shared libraries: ${enable_shared}
> + ABI compatible: ${abi_compat}
> cunit: ${cunit_support}
> test_vald: ${test_vald}
> test_perf: ${test_perf}
> 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..297a199
> --- /dev/null
> +++ b/platform/linux-generic/include/odp/api/plat/inlines.h
> @@ -0,0 +1,30 @@
> +/* Copyright (c) 2016, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +/**
> + * @file
> + *
> + * Macro for static inline functions
> + */
> +
> +#ifndef ODP_PLAT_INLINES_H_
> +#define ODP_PLAT_INLINES_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#if ODP_ABI_COMPAT == 0
> +#define _STATIC static inline
> +#else
> +#define _STATIC
> +#endif
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#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
>
>
