Skip to content

Fix SIMD fallback code when AVX2 is not available #228

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix SIMD fallback code when AVX2 is not available #228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aaad539
fix builds when 256-bit vectors are not available
j-schultz Jun 4, 2024
27a25bb
add basic .gitignore
j-schultz Jun 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/models
/src/rnnoise_data.c
/src/rnnoise_data.h
/src/rnnoise_data_little.c
/src/rnnoise_data_little.h
2 changes: 1 addition & 1 deletion src/vec_avx.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ typedef struct {
__m128i lo;
__m128i hi;
} mm256i_emu;
typedef __m256i real_m256i;
#define __m256i mm256i_emu
typedef __m256i real_m256i;
Copy link

Choose a reason for hiding this comment

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

Doesn't this simply become typedef mm256i_emu real_m256i after preprocessing?

Copy link
Contributor Author

@j-schultz j-schultz Jun 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, but mm256i_emu is also used in other places. I just wanted to fix the code with the least changes possible to make it easy to review.

Copy link

@johncf johncf Jun 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

But if both are always the same type, there wouldn't be any need to define this union (which is only relevant when __AVX2__ is not defined but __AVX__ is).

Note: I'm not familiar with the codebase, just started exploring for a project. Just pointing out a logical inconsistency.


static inline mm256i_emu mm256_setzero_si256(void) {
mm256i_emu ret;
Expand Down