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

fix: MSVC SIMD compiling #3149

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

EduMenges
Copy link
Contributor

@EduMenges EduMenges commented Jan 3, 2025

I have found issues when compiling the SIMD targets for MSVC.

Firstly, the target MNNAVX requires the AVX2 instruction set. While this is correctly set when using Clang:

target_compile_options(MNNAVX PRIVATE -mavx2 -DMNN_X86_USE_ASM)

It is not correct when using MSVC, which is still set to AVX. This prevents correct compiling.

The same is true for the MNNSSE target, which requires the AVX arch (the SSE arch is ignored in x64).

Lastly, there is a weird 256 bit dereference as if it is a struct, but in MSVC it is an opaque type, thus it should be dereferenced using SIMD intrinsics. I've updated the code so that the GCC/MSVC distinction is not necessary.

The fact that MNN's CI for Windows does not catch these errors makes me think the SIMD parts are not getting compiled during CI.

@EduMenges EduMenges changed the title fix: MSVC needs AVX2 arch fix: MSVC SIMD compiling Jan 3, 2025
@jxt1234
Copy link
Collaborator

jxt1234 commented Jan 10, 2025

What's your version of Vision Studio. It seems not meet compile error for 2017 and later.

@EduMenges
Copy link
Contributor Author

What's your version of Vision Studio. It seems not meet compile error for 2017 and later.

It's the 2022 Community Edition, more precisely version 17.12.3.

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