Skip to content

Fix avx512 build error from clang#553

Merged
JohanMabille merged 1 commit into
xtensor-stack:masterfrom
cyb70289:clang-avx512
Sep 9, 2021
Merged

Fix avx512 build error from clang#553
JohanMabille merged 1 commit into
xtensor-stack:masterfrom
cyb70289:clang-avx512

Conversation

@cyb70289
Copy link
Copy Markdown
Contributor

@cyb70289 cyb70289 commented Sep 8, 2021

I'm building apache arrow with latest xsimd and met with many errors
when using clang and enabling avx512. This patch fixes the issues.

uint64_t mask_low1 = _mm512_cmp_epu32_mask(batch<uint32_t, A>(self.data) & batch<uint32_t, A>(0x0000FF00), batch<uint32_t, A>(other.data) & batch<uint32_t, A>(0x0000FF00), Cmp);
uint64_t mask_high0 = _mm512_cmp_epu32_mask(batch<uint32_t, A>(self.data) & batch<uint32_t, A>(0x00FF0000), batch<uint32_t, A>(other.data) & batch<uint32_t, A>(0x00FF0000), Cmp);
uint64_t mask_high1 = _mm512_cmp_epu32_mask(batch<uint32_t, A>(self.data) & batch<uint32_t, A>(0xFF000000), batch<uint32_t, A>(other.data) & batch<uint32_t, A>(0xFF000000), Cmp);
uint64_t mask_low0 = _mm512_cmp_epu32_mask((batch<uint32_t, A>(self.data) & batch<uint32_t, A>(0x000000FF)), (batch<uint32_t, A>(other.data) & batch<uint32_t, A>(0x000000FF)), Cmp);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_mm512_cmp_xxx_mask are macros in clang. The , between template augments confuses the compiler.
Adding parentheses fixes the issue.

Comment thread include/xsimd/config/xsimd_config.hpp Outdated
Comment on lines 131 to 136
#if defined(__clang__) && __clang_major__ >= 6
#define XSIMD_WITH_AVX512F 1
#else
// AVX512 instructions are supported starting with gcc 6
// see https://www.gnu.org/software/gcc/gcc-6/changes.html
#if defined(__GNUC__) && __GNUC__ < 6
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang also defines __GNUC__, but looks it's fixed to 4, even for latest clang-12 [1]. So avx512 is always disabled when build xsimd with clang.
Add explicit clang checking to fix the issue.
[1] https://godbolt.org/z/n6cEexPfn

@cyb70289
Copy link
Copy Markdown
Contributor Author

cyb70289 commented Sep 8, 2021

@pitrou

Comment thread include/xsimd/config/xsimd_config.hpp Outdated
I'm building apache arrow with latest xsimd and met with many errors
when using clang and enabling avx512. This patch fixes the issues.
@pitrou
Copy link
Copy Markdown
Contributor

pitrou commented Sep 9, 2021

Do we know why CI didn't catch this on master?

@JohanMabille
Copy link
Copy Markdown
Member

@cyb70289 thanks for the fixes!
@pitrou I think we test avx512 with gcc only, I will add a build ofr testing it with clang.

@JohanMabille JohanMabille merged commit a4d01ff into xtensor-stack:master Sep 9, 2021
@cyb70289 cyb70289 deleted the clang-avx512 branch September 9, 2021 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants