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

Disable avx512 default #542

Merged
merged 3 commits into from
Dec 29, 2021
Merged

Conversation

Vindaar
Copy link
Collaborator

@Vindaar Vindaar commented Dec 29, 2021

This finally fixes #505, even if not in the most elegant way.

We now disable AVX512 support by default. Users with CPUs that support it, need to supply -d:avx512 as a compilation flag.

The reasons for this are explained in the comments to #505, but the short version is:

  • the generated C code is invalid without supplying -mavx512dq to gcc / clang
  • if -mavx512dq is supplied, the resulting binary forces the usage of AVX512 on all CPUs, resulting in illegal operation runtime errors on CPUs that do not support it.

I do not know a cleaner way to make sure the C code is generated correctly, but does not force the usage of AVX512. If anyone has a better understanding of the issue, feel free to comment or open a PR.

This commit fixes issue mratsim#505, which is a codegen issue arising from
generated C code that is only valid if the `-mavx512dq` compilation
flag is handed to the C compiler.

The issue is that:
- we cannot generate the code without the compilation flag (and
compile it successfully)
- handing the compilation flag makes the code compile, but forces
every CPU to attempt to run the AVX512 code, which is an illegal
operation for CPUs that do not support it.

For this reason AVX512 support is hidden behind a `-d:avx512`
compilation flag that needs to be handed. The resulting binary then
*will* use AVX512 for gemm.
@Vindaar Vindaar merged commit 62f1fc6 into mratsim:master Dec 29, 2021
ringabout added a commit to nim-lang/Nim that referenced this pull request Jan 3, 2022
The cause of arraymancer failure has been tracked here: mratsim/Arraymancer#505
And it was fixed by mratsim/Arraymancer#542
ringabout added a commit to nim-lang/Nim that referenced this pull request Jan 3, 2022
The cause of arraymancer failure has been tracked here: mratsim/Arraymancer#505
And it was fixed by mratsim/Arraymancer#542
@ringabout
Copy link
Contributor

Since nim-lang/Nim#19363 is merged to 1.6 branch, avx512 can be enabled.

PMunch pushed a commit to PMunch/Nim that referenced this pull request Mar 28, 2022
The cause of arraymancer failure has been tracked here: mratsim/Arraymancer#505
And it was fixed by mratsim/Arraymancer#542
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.

make arraymancer work with devel
2 participants