[PATCH] Add option to allow disabling inlined xxhash

[ChenQi]

On 2/19/25 00:18, Mark Wielaard wrote:
> Hi,
>
> On Mon, 2025-02-17 at 23:10 -0800, Qi.Chen@windriver.com wrote:
>> From: Chen Qi <Qi.Chen@windriver.com>
>>
>> xxhash cannot always inline. For example, when using gcc14 and
>> '-Og' option, xxhash cannot inline. See links below:
>> https://github.com/Cyan4973/xxHash/commit/574aabad87b2ab9440403e92e1075ef48554eb87
>> https://github.com/Cyan4973/xxHash/issues/943#issuecomment-2563205130
>>
>> To allow users successfully build debugedit with gcc14 and "-Og" option,
>> add an option to allow disabling inlined xxhash.
>>
>> This patch refers to a similar patch for libabigail:
>> https://sourceware.org/cgit/libabigail/commit/?id=50497911e2590c21270e0763d277457cf7752c3f
>>
>> Note that the default remains using inlined xxhash.
> I like this patch, but have one comment/question below.
>> diff --git a/Makefile.am b/Makefile.am
>> index 2c8c5c2..2c09e10 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -42,8 +42,8 @@ find-debuginfo: $(top_srcdir)/scripts/find-debuginfo.in Makefile
>>   
>>   debugedit_SOURCES = tools/debugedit.c \
>>   		    tools/hashtab.c
>> -debugedit_CFLAGS = @LIBELF_CFLAGS@ @LIBDW_CFLAGS@ $(AM_CFLAGS)
>> -debugedit_LDADD = @LIBELF_LIBS@ @LIBDW_LIBS@
>> +debugedit_CFLAGS = @LIBELF_CFLAGS@ @LIBDW_CFLAGS@ @XXHASH_CFLAGS@ $(AM_CFLAGS)
>> +debugedit_LDADD = @LIBELF_LIBS@ @LIBDW_LIBS@ @XXHASH_LIBS@
>>   
>>   sepdebugcrcfix_SOURCES = tools/sepdebugcrcfix.c
>>   sepdebugcrcfix_CFLAGS = @LIBELF_CFLAGS@ $(AM_CFLAGS)
> This part looks correct, and I think XXHASH_CFLAGS should have been
> there from the start.
>
>> diff --git a/configure.ac b/configure.ac
>> index a5a6e28..1421293 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -181,6 +181,17 @@ else
>>   fi
>>   AC_SUBST([READELF_VERSION_OK])
>>   
>> +ENABLE_INLINED_XXHASH=yes
>> +AC_ARG_ENABLE(inlined-xxhash,
>> +	AC_HELP_STRING([--disable-inlined-xxhash], [disable the inlined-only version of xxhash library]),
>> +	ENABLE_INLINED_XXHASH=$enableval,
>> +	ENABLE_INLINED_XXHASH=yes)
>> +
>> +if test x$ENABLE_INLINED_XXHASH = xyes; then
>> +	XXHASH_CFLAGS="-DXXH_INLINE_ALL"
>> +	XXHASH_LIBS=""
>> +fi
>> +
> So XXHASH_CFLAGS and XXHASH_LIBS have been setup above by
> PKG_CHECK_MODULES([XXHASH], [libxxhash >= 0.8.0])
>
> Clearing XXHASH_LIBS seems correct.
>
> But XXHASH_CFLAGS could include some flags like -I... already.
> So shouldn't this be:
>
> XXHASH_CFLAGS="$XXHASH_CFLAGS -DXXH_INLINE_ALL"

Yes. You're right. Though the XXHASH_CFLAGS is empty for now, we'd 
better keep it in case things change in the future.

I'll send out V2.

Regards,

Qi

>
>>   # And generate the output files.
>>   AC_CONFIG_FILES([Makefile])
>>   AC_OUTPUT
>> diff --git a/tools/debugedit.c b/tools/debugedit.c
>> index beefd65..43f9cee 100644
>> --- a/tools/debugedit.c
>> +++ b/tools/debugedit.c
>> @@ -82,7 +82,6 @@ typedef struct Ebl_Strtab	Strtab;
>>   
>>   #include "tools/hashtab.h"
>>   
>> -#define XXH_INLINE_ALL
>>   #include "xxhash.h"
>>   
>>   #define DW_TAG_partial_unit 0x3c
> OK. XXH_INLINE_ALL is now included in the CFLAGS.
>
> Thanks,
>
> Mark
>