| Message ID | 1480537973-7830-2-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | Accepted |
| Commit | 2f78516616ba40cfa316c739816ef34ef7924a49 |
| Headers | show |
On 30 November 2016 at 15:32, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > original configure.ac enables abi compat mode by default, > even without --enable-abi-compat provided. And has broken > logic to disable abi compat mode. Correct logic to build abi > compat mode and option to disable it. Shared library is not > needed for non abi compat mode, so turn it off. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > Reviewed-by: Mike Holmes <mike.holmes@linaro.org> > --- > configure.ac | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/configure.ac b/configure.ac > index be5a292..b460a65 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -241,13 +241,11 @@ ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG=$ODP_DEBUG" > 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 > + [ --disable-abi-compat disables ABI compatible mode, enables > inline code in header files], > + [if test "x$enableval" = "xno"; then > ODP_ABI_COMPAT=0 > abi_compat=no > + enable_shared=no > fi]) > AC_SUBST(ODP_ABI_COMPAT) > > @@ -336,6 +334,7 @@ AC_MSG_RESULT([ > static libraries: ${enable_static} > shared libraries: ${enable_shared} > ABI compatible: ${abi_compat} > + ODP_ABI_COMPAT: ${ODP_ABI_COMPAT} > cunit: ${cunit_support} > test_vald: ${test_vald} > test_perf: ${test_perf} > -- > 2.7.1.250.gff4ea60 > > -- Mike Holmes Program Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs "Work should be fun and collaborative, the rest follows"
Merged. On 12/01/16 21:37, Mike Holmes wrote: > > > On 30 November 2016 at 15:32, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > original configure.ac <http://configure.ac> enables abi compat mode > by default, > even without --enable-abi-compat provided. And has broken > logic to disable abi compat mode. Correct logic to build abi > compat mode and option to disable it. Shared library is not > needed for non abi compat mode, so turn it off. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> > > > Reviewed-by: Mike Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> > > > --- > configure.ac <http://configure.ac> | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/configure.ac <http://configure.ac> b/configure.ac > <http://configure.ac> > index be5a292..b460a65 100644 > --- a/configure.ac <http://configure.ac> > +++ b/configure.ac <http://configure.ac> > @@ -241,13 +241,11 @@ ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG=$ODP_DEBUG" > 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 > + [ --disable-abi-compat disables ABI compatible mode, > enables inline code in header files], > + [if test "x$enableval" = "xno"; then > ODP_ABI_COMPAT=0 > abi_compat=no > + enable_shared=no > fi]) > AC_SUBST(ODP_ABI_COMPAT) > > @@ -336,6 +334,7 @@ AC_MSG_RESULT([ > static libraries: ${enable_static} > shared libraries: ${enable_shared} > ABI compatible: ${abi_compat} > + ODP_ABI_COMPAT: ${ODP_ABI_COMPAT} > cunit: ${cunit_support} > test_vald: ${test_vald} > test_perf: ${test_perf} > -- > 2.7.1.250.gff4ea60 > > > > > -- > Mike Holmes > Program Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/>* **│ *Open source software for ARM SoCs > "Work should be fun and collaborative, the rest follows" > > __ > >
> -----Original Message----- > From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of Maxim > Uvarov > Sent: Thursday, December 01, 2016 9:50 PM > To: Mike Holmes <mike.holmes@linaro.org> > Cc: lng-odp <lng-odp@lists.linaro.org> > Subject: Re: [lng-odp] [PATCHv10 1/3] configure.ac: disable shared library > for non abi compat mode > > Merged. > > On 12/01/16 21:37, Mike Holmes wrote: > > > > > > On 30 November 2016 at 15:32, Maxim Uvarov <maxim.uvarov@linaro.org > > <mailto:maxim.uvarov@linaro.org>> wrote: > > > > original configure.ac <http://configure.ac> enables abi compat mode > > by default, > > even without --enable-abi-compat provided. And has broken > > logic to disable abi compat mode. Correct logic to build abi > > compat mode and option to disable it. Shared library is not > > needed for non abi compat mode, so turn it off. What is "broken logic" with non-ABI compatible shared lib? Why I could not build shared ODP libs that are used locally on my target CPU/SoC. I'd share the lib between multiple applications (ODP instances) for e.g. improved instruction cache foot print. I'd still want best performance which means inlined functions and implementation specific types (== non-ABI compatibility). Non-ABI compatibility is perfectly OK for me since I'm building the libs and apps locally, and not going to distribute those to other systems. So, static/shared/abi-compat are three different choice I should have. ABI-compat is needed for shared libs, when building for package management / distros, but there are also user that build apps and libs locally. > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org > > <mailto:maxim.uvarov@linaro.org>> > > > > > > Reviewed-by: Mike Holmes <mike.holmes@linaro.org > > <mailto:mike.holmes@linaro.org>> > > > > > > --- > > configure.ac <http://configure.ac> | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/configure.ac <http://configure.ac> b/configure.ac > > <http://configure.ac> > > index be5a292..b460a65 100644 > > --- a/configure.ac <http://configure.ac> > > +++ b/configure.ac <http://configure.ac> > > @@ -241,13 +241,11 @@ ODP_CFLAGS="$ODP_CFLAGS - > DODP_DEBUG=$ODP_DEBUG" > > 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 > > + [ --disable-abi-compat disables ABI compatible mode, > > enables inline code in header files], > > + [if test "x$enableval" = "xno"; then > > ODP_ABI_COMPAT=0 > > abi_compat=no > > + enable_shared=no > > fi]) > > AC_SUBST(ODP_ABI_COMPAT) > > > > @@ -336,6 +334,7 @@ AC_MSG_RESULT([ > > static libraries: ${enable_static} > > shared libraries: ${enable_shared} > > ABI compatible: ${abi_compat} > > + ODP_ABI_COMPAT: ${ODP_ABI_COMPAT} Why ABI compat needs to be printed on two different rows? To me the patch should be reverted altogether. -Petri
On 12/02/16 11:29, Savolainen, Petri (Nokia - FI/Espoo) wrote: > What is "broken logic" with non-ABI compatible shared lib? configure.ac has ABI-compat enabled and at the same time --enable-abi-compat option. I.e. enable what is already enabled. So change is to keep it enabled by default and change to --disable option. Why I could not build shared ODP libs that are used locally on my target CPU/SoC. I'd share the lib between multiple applications (ODP instances) for e.g. improved instruction cache foot print. shared library for function with inlines does not make any reason due to inclines and types of inlines can be changed. And just upgrade such .so might cause problems. > I'd still want best performance which means inlined functions and implementation specific types (== non-ABI compatibility). ok, just recompile with static library. > Non-ABI compatibility is perfectly OK for me since I'm building the libs and apps locally, and not going to distribute those to other systems. > For non-ABI (inlines) so will no be build. For ABI mode we build both .a and .so > So, static/shared/abi-compat are three different choice I should have. ABI-compat is needed for shared libs, when building for package management / distros, but there are also user that build apps and libs locally. > by default (no flags to configure) ABI-compat static and shared libraries are build. because this patch had several versions and was merged please check final commit in master branch. Best regards, Maxim.
> -----Original Message----- > From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] > Sent: Friday, December 02, 2016 3:42 PM > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell- > labs.com>; Mike Holmes <mike.holmes@linaro.org> > Cc: lng-odp <lng-odp@lists.linaro.org> > Subject: Re: [lng-odp] [PATCHv10 1/3] configure.ac: disable shared library > for non abi compat mode > > On 12/02/16 11:29, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > What is "broken logic" with non-ABI compatible shared lib? > > configure.ac has ABI-compat enabled and at the same time > --enable-abi-compat option. I.e. enable what is already enabled. So > change is to > keep it enabled by default and change to --disable option. > > > Why I could not build shared ODP libs that are used locally on my > target CPU/SoC. I'd share the lib between multiple applications (ODP > instances) for e.g. improved instruction cache foot print. > > shared library for function with inlines does not make any reason due to > inclines and types of inlines can be changed. And just upgrade such .so > might cause problems. One reason is mentioned above. Potentially improved (instruction) cache utilization. When the same ODP lib code is linked against all (tens of running ODP) applications, the same code/static data of the ODP lib is stored only once into various cache levels. E.g. 1MB cache usage vs 10MB may have a big impact on overall system performance. > > > I'd still want best performance which means inlined functions and > implementation specific types (== non-ABI compatibility). > > ok, just recompile with static library. There may be many reasons why I just cannot use static library (tools, cache performance, etc). I'm building locally, so no reason why libs should be ABI compatible. > > > Non-ABI compatibility is perfectly OK for me since I'm building the libs > and apps locally, and not going to distribute those to other systems. > > > > For non-ABI (inlines) so will no be build. For ABI mode we build both .a > and .so I need that .so with the best possible performance... > > > > So, static/shared/abi-compat are three different choice I should have. > ABI-compat is needed for shared libs, when building for package management > / distros, but there are also user that build apps and libs locally. > > > > by default (no flags to configure) ABI-compat static and shared > libraries are build. > > because this patch had several versions and was merged please check > final commit in master branch. The idea of ABI compat mode is to control static/shared/ABI-mode separately. If I need to use shared libs and build against certain HW (e.g. ARM SoC), I should be able to get the best performance of that platform and not be forced into limitations of ABI compatibility. ABI compat means trading off performance to binary compatibility and that may be costly when SoCs are very different. -Petri
ok I sent patch to enable it. Please review. If it can be applied after current release let me know. To not wait again 24 hows for that. Maxim. On 12/02/16 17:21, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -----Original Message----- >> From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org] >> Sent: Friday, December 02, 2016 3:42 PM >> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell- >> labs.com>; Mike Holmes <mike.holmes@linaro.org> >> Cc: lng-odp <lng-odp@lists.linaro.org> >> Subject: Re: [lng-odp] [PATCHv10 1/3] configure.ac: disable shared library >> for non abi compat mode >> >> On 12/02/16 11:29, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>> What is "broken logic" with non-ABI compatible shared lib? >> >> configure.ac has ABI-compat enabled and at the same time >> --enable-abi-compat option. I.e. enable what is already enabled. So >> change is to >> keep it enabled by default and change to --disable option. >> >> >> Why I could not build shared ODP libs that are used locally on my >> target CPU/SoC. I'd share the lib between multiple applications (ODP >> instances) for e.g. improved instruction cache foot print. >> >> shared library for function with inlines does not make any reason due to >> inclines and types of inlines can be changed. And just upgrade such .so >> might cause problems. > > One reason is mentioned above. Potentially improved (instruction) cache utilization. When the same ODP lib code is linked against all (tens of running ODP) applications, the same code/static data of the ODP lib is stored only once into various cache levels. E.g. 1MB cache usage vs 10MB may have a big impact on overall system performance. > >> >>> I'd still want best performance which means inlined functions and >> implementation specific types (== non-ABI compatibility). >> >> ok, just recompile with static library. > > There may be many reasons why I just cannot use static library (tools, cache performance, etc). I'm building locally, so no reason why libs should be ABI compatible. > >> >>> Non-ABI compatibility is perfectly OK for me since I'm building the libs >> and apps locally, and not going to distribute those to other systems. >>> >> >> For non-ABI (inlines) so will no be build. For ABI mode we build both .a >> and .so > > I need that .so with the best possible performance... > >> >> >>> So, static/shared/abi-compat are three different choice I should have. >> ABI-compat is needed for shared libs, when building for package management >> / distros, but there are also user that build apps and libs locally. >>> >> >> by default (no flags to configure) ABI-compat static and shared >> libraries are build. >> >> because this patch had several versions and was merged please check >> final commit in master branch. > > > The idea of ABI compat mode is to control static/shared/ABI-mode separately. If I need to use shared libs and build against certain HW (e.g. ARM SoC), I should be able to get the best performance of that platform and not be forced into limitations of ABI compatibility. ABI compat means trading off performance to binary compatibility and that may be costly when SoCs are very different. > > > -Petri >
diff --git a/configure.ac b/configure.ac index be5a292..b460a65 100644 --- a/configure.ac +++ b/configure.ac @@ -241,13 +241,11 @@ ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG=$ODP_DEBUG" 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 + [ --disable-abi-compat disables ABI compatible mode, enables inline code in header files], + [if test "x$enableval" = "xno"; then ODP_ABI_COMPAT=0 abi_compat=no + enable_shared=no fi]) AC_SUBST(ODP_ABI_COMPAT) @@ -336,6 +334,7 @@ AC_MSG_RESULT([ static libraries: ${enable_static} shared libraries: ${enable_shared} ABI compatible: ${abi_compat} + ODP_ABI_COMPAT: ${ODP_ABI_COMPAT} cunit: ${cunit_support} test_vald: ${test_vald} test_perf: ${test_perf}
original configure.ac enables abi compat mode by default, even without --enable-abi-compat provided. And has broken logic to disable abi compat mode. Correct logic to build abi compat mode and option to disable it. Shared library is not needed for non abi compat mode, so turn it off. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- configure.ac | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) -- 2.7.1.250.gff4ea60