Can't compile on CentOS 7

willbryant commented

2020-10-18 04:53:29 -05:00

(Migrated from github.com)

I'm trying to do best-efforts compilation of 0.8.0 including the X86 dispatch code for use in my database synchronization software.

CentOS 7 has GCC 4.8.5 which doesn't support AVX-512, that is, it doesn't support the -mavx512f and -mavx512vl flags and gives errors like error: attribute(target("avx512f")) is unknown, error: '__m512i' was not declared in this scope etc. without them.

As a result, I can't compile the dispatch code, because it will turn on compilation of all vectorized variants:

#if (XXH_VECTOR == XXH_AVX512) || defined(XXH_X86DISPATCH)

We could mix in more of the __AVX512F__ define tests, but I suspect that's going to require compiler-specific stuff to be spread throughout the file. Instead I propose that we refactor this to a set of vectorization support flags (eg. XXH_VECTOR_SUPPORT_AVX512), which can be left unset when the compiler flag tests fail.

(I don't use CentOS and I don't really get why people do. But they do, and they sent me bug reports and patches for my open-source software to support it.)

I'm trying to do best-efforts compilation of 0.8.0 including the X86 dispatch code for use in my database synchronization software. CentOS 7 has GCC 4.8.5 which doesn't support AVX-512, that is, it doesn't support the `-mavx512f` and `-mavx512vl` flags and gives errors like `error: attribute(target("avx512f")) is unknown`, `error: '__m512i' was not declared in this scope` etc. without them. As a result, I can't compile the dispatch code, because it will turn on compilation of all vectorized variants: `#if (XXH_VECTOR == XXH_AVX512) || defined(XXH_X86DISPATCH)` We could mix in more of the `__AVX512F__` define tests, but I suspect that's going to require compiler-specific stuff to be spread throughout the file. Instead I propose that we refactor this to a set of vectorization support flags (eg. `XXH_VECTOR_SUPPORT_AVX512`), which can be left unset when the compiler flag tests fail. (I don't use CentOS and I don't really get why people do. But they do, and they sent me bug reports and patches for my open-source software to support it.)

👍 1

Cyan4973 commented

2020-10-18 08:06:17 -05:00

(Migrated from github.com)

That's a good point @willbryant .

We do have one mechanism for such situation, though it is currently code-driven :
disable the definition of XXH_DISPATCH_AVX512.
This should disable any trace of AVX512 code generation.

Try to test if commenting this line solves your problem.
If it does, then we could leverage this mechanism to make this feature selectable at compile time,
and then make the build system able to automatically detect and trigger the feature.

That's a good point @willbryant . We do have one mechanism for such situation, though it is currently code-driven : disable the definition of [`XXH_DISPATCH_AVX512`](https://github.com/Cyan4973/xxHash/blob/dev/xxh_x86dispatch.c#L63). This should disable any trace of `AVX512` code generation. Try to test if commenting this line solves your problem. If it does, then we could leverage this mechanism to make this feature selectable at compile time, and then make the build system able to automatically detect and trigger the feature.

easyaspi314 commented

2020-10-18 12:26:56 -05:00

(Migrated from github.com)

Perhaps we could add a check before defining it?

I am thinking this

#ifdef __has_include
#  define XXH_HAS_INCLUDE(header) __has_include(header)
#else 
#  define XXH_HAS_INCLUDE(header) 0
#endif

#ifndef XXH_DISPATCH_AVX512
/*
 * Try to check if the compiler supports AVX512.
 *
 * There are a few things we can check:
 *   - While they don't let you directly include the header,
 *    GCC and Clang both have an internal header called
 *    "avx512fintrin.h" on the built-in include path.
 *    We can detect both GCC and Clang this way.
 *  - GCC 4.9 and later are known to support AVX512.
 *  - Visual Studio 2017 also supports AVX512.
 */
#  if XXH_HAS_INCLUDE(<avx512fintrin.h>) /* GCC/Clang internal header */ \
     || (defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 9))) /* GCC 4.9+ */ \
     || (defined(_MSC_VER) && _MSC_VER >= 1910) /* VS 2017+ */
#    define XXH_DISPATCH_AVX512 1
#  else
#    define XXH_DISPATCH_AVX512 0
#  endif
#endif

Perhaps we could add a check before defining it? I am thinking this ```c #ifdef __has_include # define XXH_HAS_INCLUDE(header) __has_include(header) #else # define XXH_HAS_INCLUDE(header) 0 #endif #ifndef XXH_DISPATCH_AVX512 /* * Try to check if the compiler supports AVX512. * * There are a few things we can check: * - While they don't let you directly include the header, * GCC and Clang both have an internal header called * "avx512fintrin.h" on the built-in include path. * We can detect both GCC and Clang this way. * - GCC 4.9 and later are known to support AVX512. * - Visual Studio 2017 also supports AVX512. */ # if XXH_HAS_INCLUDE(<avx512fintrin.h>) /* GCC/Clang internal header */ \ || (defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 9))) /* GCC 4.9+ */ \ || (defined(_MSC_VER) && _MSC_VER >= 1910) /* VS 2017+ */ # define XXH_DISPATCH_AVX512 1 # else # define XXH_DISPATCH_AVX512 0 # endif #endif ```

easyaspi314 commented

2020-10-18 12:45:41 -05:00

(Migrated from github.com)

Apparently for AVX2, we need GCC 4.7, avx2intrin.h, or VS 2013 Update 2.

easyaspi314 commented

2020-10-18 16:14:38 -05:00

(Migrated from github.com)

@willbryant Can you test my dispatch-fixes branch?

@willbryant Can you test my `dispatch-fixes` branch?

willbryant commented

2020-10-19 03:55:34 -05:00

(Migrated from github.com)

No different at first:

xxhash.h:3691:1: error: attribute(target("avx512f")) is unknown

But what compiler options and defines are you expecting to be passed?

Here's a minimal Dockerfile extracted from my test build for Centos 7:

FROM centos:7

RUN yum update -y && yum clean all && yum install -y gcc make git

WORKDIR /tmp
#RUN git clone https://github.com/Cyan4973/xxHash.git
RUN git clone https://github.com/easyaspi314/xxHash.git --branch dispatch-fixes

WORKDIR /tmp/xxHash
RUN make
RUN gcc -msse2 -msse4.1 -mavx2 xxh_x86dispatch.c

Not sure if you would expect those options (noting the absence of -mavx512* options), no options at all, or something more like -march=native, but I think all three fail the same way right now.

No different at first: ```In file included from xxh_x86dispatch.c:175:0: xxhash.h:3691:1: error: attribute(target("avx512f")) is unknown ``` But what compiler options and defines are you expecting to be passed? Here's a minimal Dockerfile extracted from my test build for Centos 7: ```Dockerfile FROM centos:7 RUN yum update -y && yum clean all && yum install -y gcc make git WORKDIR /tmp #RUN git clone https://github.com/Cyan4973/xxHash.git RUN git clone https://github.com/easyaspi314/xxHash.git --branch dispatch-fixes WORKDIR /tmp/xxHash RUN make RUN gcc -msse2 -msse4.1 -mavx2 xxh_x86dispatch.c ``` Not sure if you would expect those options (noting the absence of -mavx512* options), no options at all, or something more like -march=native, but I think all three fail the same way right now.

easyaspi314 commented

2020-10-19 08:36:30 -05:00

(Migrated from github.com)

I see the issue now.

While I successfully disabled generating the dispatch table, I need to also disable the code path on xxhash.h.

Side note, you are supposed to compile xxh_x86dispatch.c with the minimum target options. I should add a sanity check in that file as it seems to be a common misconception.

#if defined(__AVX__)
/*
 * Do not compile xxh_x86dispatch.c with options like -mavx2 or /arch:avx2!
 *
 * Using this option globally allows this feature, and therefore makes
 * it undefined behavior to execute on any CPU without said feature.
 *
 * Even if the compiler isn't directly using AVX intrinsics in a function,
 * the compiler can still generate AVX code from autovectorization and 
 * by "upgrading" SSE2 intrinsics to use the VEX prefixes.
 * 
 * Use the same flags that you use to compile the rest of the program;
 * this file will safely generate AVX2 and AVX512 without these flags.
 */
#  warning "Do not compile xxh_x86dispatch.c with AVX globally enabled! See the comment above."
#endif

I see the issue now. While I successfully disabled generating the dispatch table, I need to also disable the code path on xxhash.h. Side note, you are supposed to compile xxh_x86dispatch.c with the *minimum* target options. I should add a sanity check in that file as it seems to be a common misconception. ```c #if defined(__AVX__) /* * Do not compile xxh_x86dispatch.c with options like -mavx2 or /arch:avx2! * * Using this option globally allows this feature, and therefore makes * it undefined behavior to execute on any CPU without said feature. * * Even if the compiler isn't directly using AVX intrinsics in a function, * the compiler can still generate AVX code from autovectorization and * by "upgrading" SSE2 intrinsics to use the VEX prefixes. * * Use the same flags that you use to compile the rest of the program; * this file will safely generate AVX2 and AVX512 without these flags. */ # warning "Do not compile xxh_x86dispatch.c with AVX globally enabled! See the comment above." #endif ```

easyaspi314 commented

2020-10-19 09:29:00 -05:00

(Migrated from github.com)

@willbryant Try now. It should also error if you use -mavx*, as that defeats the purpose of the dispatching altogether.

Just do gcc -O2 -c xxh_x86dispatch.c, macro magic will handle the rest. 😉

@willbryant Try now. It should also error if you use `-mavx*`, as that defeats the purpose of the dispatching altogether. Just do `gcc -O2 -c xxh_x86dispatch.c`, macro magic will handle the rest. 😉

willbryant commented

2020-10-19 15:42:43 -05:00

(Migrated from github.com)

Cool, no options is what I tried first, but good to prevent it altogether.

Compilation now failing with:

In file included from xxh_x86dispatch.c:208:0:
xxhash.h: In function 'XXH3_accumulate_512_avx2':
xxhash.h:3801:5: error: unknown type name '__m256i'

FROM centos:7

RUN yum update -y && yum clean all && yum install -y gcc make git

WORKDIR /tmp
#RUN git clone https://github.com/Cyan4973/xxHash.git
RUN git clone https://github.com/easyaspi314/xxHash.git --branch dispatch-fixes

WORKDIR /tmp/xxHash
RUN make
RUN gcc xxh_x86dispatch.c

Cool, no options is what I tried first, but good to prevent it altogether. Compilation now failing with: ``` In file included from xxh_x86dispatch.c:208:0: xxhash.h: In function 'XXH3_accumulate_512_avx2': xxhash.h:3801:5: error: unknown type name '__m256i' ``` ```Dockerfile FROM centos:7 RUN yum update -y && yum clean all && yum install -y gcc make git WORKDIR /tmp #RUN git clone https://github.com/Cyan4973/xxHash.git RUN git clone https://github.com/easyaspi314/xxHash.git --branch dispatch-fixes WORKDIR /tmp/xxHash RUN make RUN gcc xxh_x86dispatch.c ```

easyaspi314 commented

2020-10-19 16:31:03 -05:00

(Migrated from github.com)

Oh, I see... Damn this is dumb.

So before GCC 4.9 (specifically this commit), GCC required __AVX__, __SSE2__, etc to include the SSE headers.

That makes it impossible to implement this in a single file without some incredibly ugly hacks that mess with the internals.

Oh, I see... Damn this is dumb. So before GCC 4.9 (specifically [this commit](https://github.com/gcc-mirror/gcc/commit/97db2bf7fb10e7eb2e8224e0471b56976f133843)), GCC required `__AVX__`, `__SSE2__`, etc to include the SSE headers. That makes it impossible to implement this in a single file without some incredibly ugly hacks that mess with the internals.

willbryant commented

2020-10-19 17:09:31 -05:00

(Migrated from github.com)

Right, messy.

FWIW before seeing how you were able to implement most of the detection in the header files, my expectation was that I would need to compile both xxhash.c and xxh_x86dispatch.c, the former with all of the supported flags on (ie. -msse2 -msse4.1 -mavx2 on this distro, -msse2 -msse4.1 -mavx2 -mavx512f -mavx512vl and maybe -mavx512bw on others), the latter with no flags.

I expected xxh_x86dispatch to need to be told which variants to try and link against/dispatch to, which I would know because my build system would have had to figure out which flags to use.

For example, in BLAKE3 I set defines like BLAKE3_NO_AVX512 to turn off dispatch to the AVX-512 variant if that variant isn't compilable/linkable.

Right, messy. FWIW before seeing how you were able to implement most of the detection in the header files, my expectation was that I would need to compile both xxhash.c and xxh_x86dispatch.c, the former with all of the supported flags on (ie. `-msse2 -msse4.1 -mavx2` on this distro, `-msse2 -msse4.1 -mavx2 -mavx512f -mavx512vl` and maybe `-mavx512bw` on others), the latter with no flags. I expected xxh_x86dispatch to need to be told which variants to try and link against/dispatch to, which I would know because my build system would have had to figure out which flags to use. For example, in BLAKE3 I set defines like `BLAKE3_NO_AVX512` to turn off dispatch to the AVX-512 variant if that variant isn't compilable/linkable.

easyaspi314 commented

2020-10-19 19:59:35 -05:00

(Migrated from github.com)

Hmm. Found a CentOS machine on the compiler farm with GCC 4.8.5 (yay no recompile).

My two filthy, ugly, evil hack ideas all resulted in inlining failed in call to always_inline ‘...': target specific option mismatch

I don't know what's going to be harder, trying to get this to work or getting CentOS to update GCC.

Also, Clang 3.4 does not work; it lacks __attribute__((__target__)).

Requiring GCC to be 4.9 and Clang to be 3.8 will fix this, but it is not something I want...

Hmm. Found a CentOS machine on the compiler farm with GCC 4.8.5 (yay no recompile). My two filthy, ugly, evil hack ideas all resulted in `inlining failed in call to always_inline ‘...': target specific option mismatch` I don't know what's going to be harder, trying to get this to work or getting CentOS to update GCC. Also, Clang 3.4 does not work; it lacks `__attribute__((__target__))`. Requiring GCC to be 4.9 and Clang to be 3.8 will fix this, but it is not something I want...

easyaspi314 commented

2020-10-19 21:31:16 -05:00

(Migrated from github.com)

Welp, this looks like the most accurate check I can find. 🤢

/*
 * GCC needs to be 4.9+ and Clang needs to be 3.8+ (Xcode 7.3+).
 *
 * While both supported the `target` attribute before that, the biggest problem
 * is immintrin.h itself.
 *
 * Before this version, the intrinsics were only defined if the feature is
 * enabled globally. This means that the `target` attribute is pretty useless.
 *
 * After this, immintrin.h included _all_ intrinsics.
 */
#if defined(__clang__)
/*
 * UGLY HACK: Test for a working immintrin.h on Clang by testing for the
 * header guard of one of its subheaders.
 *
 * We can't trust version numbers for accurate versions, thanks to Xcode.
 * However, since Clang 8+ and Xcode 8+ are both ok, we can skip this test
 * in case something changes in the future.
 */
#  if __clang_major__ < 8
#    undef __AVX2__              /* Make sure __AVX2__ is not defined */
#    include <immintrin.h>       /* Include immintrin.h */
#    if !defined(__AVX2INTRIN_H) /* Check for avx2intrin.h's header guard */
#      error "Dispatching requires MSVC, ICC, GCC 4.9+, Clang 3.8+, or Xcode 7.3+."
#    endif
#  endif /* LLVM 8 or Xcode 8 */
#elif defined(__GNUC__) && !defined(__INTEL_COMPILER) \
     && !(__GNUC__ >= 5 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 9)) /* GCC 4.9+ */
/* Note: GCC has no header guard, but we can check for 4.9 directly. */
#  error "Dispatching requires MSVC, ICC, GCC 4.9+, Clang 3.8+, or Xcode 7.3+."
#endif

Welp, this looks like the most accurate check I can find. 🤢 ```c /* * GCC needs to be 4.9+ and Clang needs to be 3.8+ (Xcode 7.3+). * * While both supported the `target` attribute before that, the biggest problem * is immintrin.h itself. * * Before this version, the intrinsics were only defined if the feature is * enabled globally. This means that the `target` attribute is pretty useless. * * After this, immintrin.h included _all_ intrinsics. */ #if defined(__clang__) /* * UGLY HACK: Test for a working immintrin.h on Clang by testing for the * header guard of one of its subheaders. * * We can't trust version numbers for accurate versions, thanks to Xcode. * However, since Clang 8+ and Xcode 8+ are both ok, we can skip this test * in case something changes in the future. */ # if __clang_major__ < 8 # undef __AVX2__ /* Make sure __AVX2__ is not defined */ # include <immintrin.h> /* Include immintrin.h */ # if !defined(__AVX2INTRIN_H) /* Check for avx2intrin.h's header guard */ # error "Dispatching requires MSVC, ICC, GCC 4.9+, Clang 3.8+, or Xcode 7.3+." # endif # endif /* LLVM 8 or Xcode 8 */ #elif defined(__GNUC__) && !defined(__INTEL_COMPILER) \ && !(__GNUC__ >= 5 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 9)) /* GCC 4.9+ */ /* Note: GCC has no header guard, but we can check for 4.9 directly. */ # error "Dispatching requires MSVC, ICC, GCC 4.9+, Clang 3.8+, or Xcode 7.3+." #endif ```

Rows
Columns

Can't compile on CentOS 7 #464