[PATCH] Add option to allow disabling inlined xxhash

[Mark Wielaard]

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"

>  # 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