xxhash.h: Fix build with gcc-12

[Mingli-Yu]

Fixes: | xxhash.h:3932:1: error: inlining failed in call to 'always_inline' 'XXH3_accumulate_512_sse2': function not considered for inlining | 3932 | XXH3_accumulate_512_sse2( void* XXH_RESTRICT acc, | | ^~~~~~~~~~~~~~~~~~~~~~~~ | xxhash.h:4369:9: note: called from here | 4369 | f_acc512(acc, | | ^~~~~~~~~~~~~ | 4370 | in, | | ~~~ | 4371 | secret + n*XXH_SECRET_CONSUME_RATE); | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Mingli Yu [email protected]

[t-mat]

Hi @Mingli-Yu, thank you for sending PR. But unfortunately your PR has failed to pass our test. Also I couldn't repro your issue with gcc-12.1.

First of all, please understand that we can't accept your PR for now. Because your PR failed to pass our test. But I also must say that our test is not perfect. So feel free to point out our mistake.

---

I failed to reproduce your issue with gcc-12.1

I've tried to build xxhsum and make test with gcc-12.1 by the following commands which found at reddit.

cd

# spack is the cleanest package manager BTW.
git clone -c feature.manyFiles=true https://github.com/spack/spack.git
. spack/share/spack/setup-env.sh
spack install [email protected]
spack load [email protected]
gcc --version
# gcc (Spack GCC) 12.1.0

git clone --depth 1 https://github.com/Cyan4973/xxHash.git
cd xxHash

make
make test
CPPFLAGS="-mavx2 -DXXH_VECTOR=XXH_AVX2" make clean check
# build xxhsum and pass all tests

./xxhsum --version
# xxhsum 0.8.1 by Yann Collet
# compiled as 64-bit x86_64 + AVX2 little endian with GCC 12.1.0

During the build and test process, I don't see any warning and error. Since you've posted specific error message, I think I missed something. But honestly, I have no idea.

---

I don't understand intention of your PR.

(1) Same question again, but I don't see any warning (as an error) from these __inline__ and __attribue__((always_inline)) in the macro. How can we reproduce your issue?

(2) Why you removed __attribute__((unused))? According to the gcc manual, I think this attribute may help to ease/resolve yor situation. At least it may help our test.

unused This attribute, attached to a function, means that the function is meant to be possibly unused. GCC does not produce a warning for this function.

[jrosdahl]

Looks like @Mingli-Yu forgot to mention that the issue only shows up when using -Og.

[t-mat]

Hi @jrosdahl, thanks for the info! Now I can reproduce this issue with the following command:

CFLAGS="-Og" CC="gcc-12" make clean default  # NG

Since I have no idea so far, I just put some random commands for further investigation.

# gcc-12 -Og
CFLAGS="-Og -no-mavx2"   CC="gcc-12" make clean default  # NG (SSE2)
CFLAGS="-Og -msse2"      CC="gcc-12" make clean default  # NG (SSE2)
CFLAGS="-Og -mavx2"      CC="gcc-12" make clean default  # OK (AVX2)
CFLAGS="-Og -mhaswell"   CC="gcc-12" make clean default  # OK (AVX2)

# gcc-12 -O?
CFLAGS="-O0"             CC="gcc-12" make clean default  # OK (SSE2)
CFLAGS="-O1"             CC="gcc-12" make clean default  # OK (SSE2)
CFLAGS="-O1 -fno-inline" CC="gcc-12" make clean default  # OK (SSE2)

# gcc-11 -Og
CFLAGS="-Og"             CC="gcc-11" make clean default  # OK (SSE2)

[tristan957]

Just to add another data point, I can also confirm this. I have been investigating a little bit on my end.

[tristan957]

I have fixed the meson build in https://github.com/mesonbuild/wrapdb/pull/580, specifically https://github.com/mesonbuild/wrapdb/pull/580/commits/e9b59a653f0b77a37aabe61d555f5a449649a455.

I think this PR has to do something similar, but I am not going to touch it because I have no clue how to tell what optimization level the compiler is using in Makefiles. I haven't found a pure C preprocessor solution. I would seriously just recommend using Meson for xxhash though :).

[Cyan4973]

Indeed, -Og seems to be "a compilation mode optimized for debugging". Given the definition, I would expect this compilation mode to be rather hostile to inlining, since inlining can make debugging more difficult.

Somehow, -Og feels superficially similar to -O0 -g.

-O0 is detectable, by checking macro __NO_INLINE__ (cc @easyaspi314). For some reason, -Og on gcc-12 doesn't expose the __NO_INLINE__ macro when selected. If it's not going to respect the inline statements, it probably should.

At least, that's the situation with gcc-12. But clang on the other hand has no such problem.

I don't have access to older versions of gcc to check if the same pb happens. Will try again later, when I can access another system with more compilers.

[easyaspi314]

Hmm, this is a tricky one. GCC 12 doesn't seem to emit any different macros on -Og vs -O2.

https://github.com/gcc-mirror/gcc/commit/c952126870c92cf293d59ffb1497e402eb8fc269 might be the culprit

The problem seems to be specifically -Og preventing GCC from inlining the accumulate functions because they are accessed through function pointers.

My guess is to either disable force inline on the accumulate functions themselves or force GCC into -O3 mode if neither -O0, -Os, or -fno-inline is used, similar to how GCC is forced to do in AVX2 (which by the way the load splitting bug seems to have been fixed, might need to find the commit to disable it on newer versions)

If we can get them to be inlined naturally (such as via static inline) this might work.

[eugeneko]

I have also encountered this issue. Any workarounds except removing -Og from our compiler options?

[tristan957]

Using the Meson build will fix this issue if that is possible for you.

[t-mat]

Hi @eugeneko, please try the HEAD of dev branch or apply the following patch

• #804

Since #814 added CI test for gcc-12 -Og, we're sure that HEAD of dev branch has no problem with gcc-12 -Og.

You can also try it in your terminal

gcc-12 --version
# gcc-12 (Ubuntu 12.1.0-2ubuntu1~22.04) 12.1.0

cd
git clone https://github.com/Cyan4973/xxHash.git
cd xxHash

git branch -v
# * dev 616ca44 minor comment update

CFLAGS="-Og" CC="gcc-12" make clean default

./xxhsum --version
# xxhsum 0.8.1 by Yann Collet compiled as 64-bit x86_64 + SSE2 little endian with GCC 12.1.0

[Cyan4973]

Fixed in #804