search

simd-everywhere / simde

Implementations of SIMD instruction sets for systems which don't natively support them.
https://simd-everywhere.github.io/blog/
MIT License
2.32k stars 239 forks source link

correction of simde_mm256_sign_epi16(). #1123

Closed Proudsalsa closed 7 months ago

Proudsalsa commented 8 months ago

Hi,

There is an error in the implementation of simde_mm256_sign_epi16(). The implementation does not handle the "==0" case of the avx2 intrinsic. I didn't understand the entire test workflow but i would guess that the error is not detected because the test vector in test_simde_mm256_sign_epi16() doesn't contain any 0s.

Feel free to edit the pull request/code.

This is the Pseudo code of the _mm256_sign_epi16 intrinsic from the Intel Intrinsics guide:

FOR j := 0 to 15
    i := j*16
    IF b[i+15:i] < 0
        dst[i+15:i] := -(a[i+15:i])
    ELSE IF b[i+15:i] == 0
        dst[i+15:i] := 0
    ELSE
        dst[i+15:i] := a[i+15:i]
    FI
ENDFOR
dst[MAX:256] := 0
mr-c commented 8 months ago

Thank you @Proudsalsa ; can you add additional test cases so we don't make these mistakes again?

Proudsalsa commented 7 months ago

i added the testcases by adding an additional element to the test vectors. I hope this is fine.

I ran the tests as described in the CONTRIBUTING.md and checked if they detect an error when run with the old implementation as well.