search

mattkretz / vir-simd

improve the usage experience of std::experimental::simd (Parallelism TS 2)
GNU Lesser General Public License v3.0
22 stars 2 forks source link

Compilation error with MSVC #10

Open AlvaroFS opened 1 year ago

AlvaroFS commented 1 year ago

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 :sweat_smile: . I'll try to create a PR at least for the iota issue.

mattkretz commented 1 year ago

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! :wink:

mattkretz commented 1 year ago

See #11, better?

AlvaroFS commented 1 year ago

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

mattkretz commented 1 year ago

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.