NLnetLabs / simdzone

Fast and standards compliant DNS zone parser
BSD 3-Clause "New" or "Revised" License
63 stars 11 forks source link

Fix to detect simd instruction compile #211

Closed wcawijngaards closed 1 month ago

wcawijngaards commented 2 months ago

This fix adds try_compile to check if the -march=westmere or -march=haswell succeeds to compile simd instructions.

There is a check added for configure and for CMakeLists.txt.

k0ekk0ek commented 1 month ago

We could add additional checks, but I expect there's really something off in the Homebrew build environment (https://github.com/Homebrew/homebrew-core/pull/175796). On the other hand, it doesn't hurt to expand on the compile checks too. I'll clean the patch up a little bit.

k0ekk0ek commented 1 month ago

@wcawijngaards, I updated the checks in configure.ac. They're a little bit smaller, but mostly, if optimization flags were passed the existing checks did not work because the compiler would optimize them out. I verified with Compiler Explorer.

See the difference here.

k0ekk0ek commented 1 month ago

I still have to update CMakeLists.txt because it uses try_compile which is only available since 3.25. It's better to use check_c_source_compiles. I'll do that next.

wcawijngaards commented 1 month ago

Why did you delete the inline function from the configure test? When I tried that this reproduced the error printout nicely about mismatches preventing simd instructions. The plain use of the simd instruction may not trigger the failure condition?

wcawijngaards commented 1 month ago

You mean you want the main function to not cast the variable to void, but instead return based on the value, or perhaps printf based on the value. I think the 'inline' should not be removed there.

k0ekk0ek commented 1 month ago

The compiler optimizes it out if the result isn't used. I'll see if the inline makes a difference after the weekend.

k0ekk0ek commented 1 month ago

@wcawijngaards, I take it you're referring to errors similar to the one below wrt to you comment on inline?

   In file included from ./src/haswell/parser.c:12:
  ./src/haswell/simd.h:88:21: error: always_inline function '_mm256_loadu_si256' requires target feature 'avx', but would be inlined into function 'simd_loadu_8x64' that is compiled without support for 'avx'
    simd->chunks[0] = _mm256_loadu_si256((const __m256i *)(address));
                      ^
  ./src/haswell/simd.h:88:21: error: AVX vector return of type '__m256i' (vector of 4 'long long' values) without 'avx' enabled changes the ABI
wcawijngaards commented 1 month ago

Yes it causes the compile test to test for several features, I guess, and the inline seems to cause a different output error than the other code line. Also most of the simdzone code wants to inline, and this seems to cause different errors from the compiler for instruction feature support.

k0ekk0ek commented 1 month ago

Reading the error it is much more about the second line ...without 'avx' enabled changes the ABI(?) I think it is about not properly supporting the -march=haswell flag, which should enable AVX2. -mavx2 might solve it, but I don't have the environment to reproduce. Plus, I'm guessing, we might need to pass a flag for popcnt to be enabled, though that may also be automatic.

k0ekk0ek commented 1 month ago

I've consolidated the tests, they both use the cmake/xxx.test.c files for configure tests. I think this does the trick.