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

Compilation error with MSVC #10

Closed
AlvaroFS opened this issue May 9, 2023 · 4 comments · Fixed by #11
Closed

Compilation error with MSVC #10

AlvaroFS opened this issue May 9, 2023 · 4 comments · Fixed by #11

Comments

@AlvaroFS
Copy link
Contributor

AlvaroFS commented May 9, 2023

Hi! I was trying to add vir-simd to Conan Center PR but the CI fails with MSVC when checking for C++17.
It seems that the __cplusplus macro contains a wrong value unless we specify de /Zc:__cplusplus compiler flag. It would be easy to add that flag for the package test but that would require every user of the library to set that flag. Maybe that's what you wanted or maybe that check could be defined using the MSVC macro _MSC_VER. I'm assuming MSVC support, if that is not the case please let me know and I'll update the Conan PR accordingly.

I also spent some time trying to make iota_v work until I saw that it is only available with C++20. I think that should be in the README.

I apologize I'm only complaining 😅 . I'll try to create a PR at least for the iota issue.

@mattkretz
Copy link
Owner

mattkretz commented May 9, 2023

I have never found time to test vir-simd with MSVC. So yes, I'm sure there are bugs. At the very least the simd_benchmarking.h header will not work for MSVC. FWIW, I'd like to make it work correctly. But from my experience with supporting MSVC for Vc I fear touching MSVC again.

You're right about the README. I added several features that make use of concepts. Needs to be documented.

I'll take a look at the Conan PR. I was independently also looking into creating a Conan package. But thank you for beating me there! 😉

@mattkretz
Copy link
Owner

See #11, better?

@AlvaroFS
Copy link
Contributor Author

AlvaroFS commented May 9, 2023

Yes, thanks!
About MSVC, I'll limit the available compilers to GCC and Clang. I'll enable others when it is supported.

@mattkretz
Copy link
Owner

Sorry for skipping the __cplusplus issue.

According to https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170 vir-simd should test if _MSVC_LANG is defined, and if yes use that instead of __cplusplus.

@mattkretz mattkretz reopened this May 11, 2023
mattkretz added a commit that referenced this issue May 11, 2023
Fixes: #10

ChangeLog:

	* vir/simd.h: Prefer _MSVC_LANG over __cplusplus.
mattkretz added a commit that referenced this issue Jul 18, 2023
Fixes: #10

ChangeLog:

	* vir/simd.h: Prefer _MSVC_LANG over __cplusplus.
mattkretz added a commit that referenced this issue Jul 18, 2023
Fixes: #10

ChangeLog:

	* vir/simd.h: Prefer _MSVC_LANG over __cplusplus.
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 a pull request may close this issue.

2 participants