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

AVX512 masked load and store functions (simde_mm512_mask_{loadu,storeu}_*) are implemented incorrectly #1204

Open
JakobEnglhauser opened this issue Aug 7, 2024 · 1 comment

Comments

@JakobEnglhauser
Copy link

The masked functions should not touch memory where the mask is set to 0 at all.

Instead, the simde_mm512_mask_storeu_* functions write a 0 if the mask is set to 0. Take for example the following code:

#include <iostream>
#include "simde/x86/avx512.h"

int main() {
        double array[8] = { 1, 2, 3, 4, 5, 6, 7, 8};

        simde_mm512_mask_storeu_pd(array, 0, simde_mm512_setzero_pd());

        for (auto d : array) {
                std::cout << d << " ";
        }
        std::cout << std::endl;
        return 0;
}

Using SIMDe:

> g++ -o test -mno-avx512f test.cpp
> ./test
0 0 0 0 0 0 0 0

Using native AVX512

> g++ -o test -mavx512f test.cpp
> ./test
1 2 3 4 5 6 7 8

For the simde_mm512_mask_loadu_* the issue isn't as severe since they do correctly keep the old values in the target register, however they still load all values from the source into a temporary register which could cause segmentation faults. Take for example the following code:

#include <iostream>
#include "simde/x86/avx512.h"

int main() {
        simde__m512d a = simde_mm512_setzero_pd();

        simde_mm512_mask_loadu_pd(a, 0, nullptr);

        for (int i = 0; i < 8; ++i) {
                std::cout << a[i] << " ";
        }
        std::cout << std::endl;
        return 0;
}

Using SIMDe:

> g++ -o test -mno-avx512f test.cpp
> ./test
Segmentation fault (core dumped)

Using native AVX512:

> g++ -o test -mavx512f test.cpp
> ./test
0 0 0 0 0 0 0 0
@mr-c
Copy link
Collaborator

mr-c commented Sep 23, 2024

@Jakob-en Thank you for your report, this makes sense and I see how we missed this in testing (we initialized to all zeros and also some functions aren't tested).

Are you able to contribute a fix?

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

No branches or pull requests

2 participants