Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing AVX-512 code from the headers. #602

Closed
wants to merge 1 commit into from
Closed

Conversation

lemire
Copy link
Member

@lemire lemire commented Mar 1, 2024

This is an attempt at fixing our illegal-instruction issue with the recent versions of Visual Studio.

We think it is a bug in Visual Studio whereas if a function might use AVX-512 in one code path, then all code paths get to use AVX-512 instructions... This means that routines such as...

if(avx512_is_supported) {
// run AVX-512 code
} else {
// run other code
}

are broken...

See RoaringBitmap/croaring-rs#128

cc @Dr-Emann

@lemire
Copy link
Member Author

lemire commented Mar 1, 2024

Ok. I am going to try to disable AVX-512 under visual studio.

@Dr-Emann
Copy link
Member

Dr-Emann commented Mar 1, 2024

I still don't think that will help: in this godbolt, we use only avx2 intrinsics, and configure the compiler with /arch:AVX2. There are no avx512 intrinsics in other code paths, there are no avx512 intrinsics at all, but the compiler outputs a vpxord instruction, which is AVX512 only. The bug sure seems like it's:

If AVX2 instructions are used, the compiler is incorrectly assuming that AVX512 is also allowed, and sometimes optimizations produce AVX512-only instructions.

I really believe the only solution is either disable AVX2 with this specific compiler version, or fiddle around with the code and inspect the assembly output, and see if we can find an incantation that accidentally avoids any optimization attempts the compiler thinks it can use avx512 for.

@lemire
Copy link
Member Author

lemire commented Mar 1, 2024

Closing.

@lemire lemire closed this Mar 1, 2024
@Dr-Emann Dr-Emann deleted the avx512_off_headers branch March 5, 2024 01:24
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.

2 participants