[PATCH] Use XXH_INLINE_ALL=1 to inline all xxhash functions

[Sam James]

Sam James <sam@gentoo.org> writes:

> Mark Wielaard <mark@klomp.org> writes:
>
>> XXH_INLINE_ALL: Make all functions inline, implementation is directly
>> included within xxhash.h. This makes it so that there is no runtime
>> dependency on libxxhash.so and could result in faster code.
>
> Why is this desirable? I don't think the faster code bit is a given --
> we could easily end up with a (much) larger binary, for example.
>
> Having xxhash essentially bundled in all packages by forcing inlining
> isn't a win IMO. If it's been measured to provide substantial benefit
> for libabigail, then that's different (but please say so!), but I don't
> think we should default to doing this whenever xxhash is used.

Reading https://github.com/Cyan4973/xxHash/issues/902 and
https://github.com/Cyan4973/xxHash/pull/903, it looks like a mess and
it's unclear whether it even guarantees that you *must* use the inline
version or whether it just gives the option (inline vs force inline).

That would mean that dropping the linkage is wrong and we should rely on
as-needed instead, at least.

>
>>
>> 	   * configure: Don't AC_SUBST XXHASH_{VERSION,LIBS,CFLAGS}.
>> 	   Remove XXHASH_LIBS from DEPS_LIBS. Add -DXXH_INLINE_ALL=1
>> 	   to CFLAGS and CXXFLAGS.
>>
>> Signed-off-by: Mark Wielaard <mark@klomp.org>
>> ---
>>
>> https://code.wildebeest.org/git/user/mjw/libabigail/commit/?h=xxhash-inline-all
>>
>>  configure.ac | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 07cebf32e46b..6c1a7ce6b85a 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -495,10 +495,6 @@ dnl Check for dependency: xxhash
>>  XXHASH_VERSION=0.8.0
>>  PKG_CHECK_MODULES(XXHASH, libxxhash >= $XXHASH_VERSION)
>>  
>> -AC_SUBST(XXHASH_VERSION)
>> -AC_SUBST(XXHASH_LIBS)
>> -AC_SUBST(XXHASH_CFLAGS)
>> -
>>  dnl Check for some programs like rm, mkdir, etc ...
>>  AC_CHECK_PROG(HAS_RM, rm, yes, no)
>>  if test x$HAS_RM = xno; then
>> @@ -922,7 +918,7 @@ AX_VALGRIND_CHECK
>>  
>>  dnl Set the list of libraries libabigail depends on
>>  
>> -DEPS_LIBS="$XML_LIBS $ELF_LIBS $DW_LIBS $XXHASH_LIBS $CTF_LIBS $BPF_LIBS"
>> +DEPS_LIBS="$XML_LIBS $ELF_LIBS $DW_LIBS $CTF_LIBS $BPF_LIBS"
>>  AC_SUBST(DEPS_LIBS)
>>  
>>  if test x$ABIGAIL_DEVEL != x; then
>> @@ -960,6 +956,10 @@ if test x$ENABLE_UBSAN = xyes; then
>>      CXXFLAGS="$CXXFLAGS -fsanitize=undefined"
>>  fi
>>  
>> +dnl We always want all xxhash functions inlined
>> +CFLAGS="$CFLAGS -DXXH_INLINE_ALL=1"
>> +CXXFLAGS="$CXXFLAGS -DXXH_INLINE_ALL=1"
>> +
>>  dnl Set a few Automake conditionals
>>  
>>  AM_CONDITIONAL([CTF_READER],[test "x$ENABLE_CTF" = "xyes"])