fix fp16 with msvc #333
fix fp16 with msvc #333
Conversation
mshadow/half.h
Outdated
| @@ -10,7 +10,11 @@ | |||
| #include "./base.h" | |||
|
|
|||
| #if MSHADOW_USE_F16C | |||
| #include <x86intrin.h> | |||
There was a problem hiding this comment.
Users also report that this breaks android build
There was a problem hiding this comment.
wont amalgamation test in mxnet ci test for this ?
There was a problem hiding this comment.
Could you point me to the issue for this if it exists?
There was a problem hiding this comment.
Android build should be using USE_F16C=0 as this feature wouldn't be supported for that hardware. Similar to how they need to turn off USE_SSE
mshadow/half.h
Outdated
| @@ -10,7 +10,11 @@ | |||
| #include "./base.h" | |||
|
|
|||
| #if MSHADOW_USE_F16C | |||
| #include <x86intrin.h> | |||
| #if defined(_MSC_VER) | |||
| #include <intrin.h> | |||
There was a problem hiding this comment.
You had mentioned in your PR description that msvc doesn't have intrin.h. If so why would we want to include this? Especially given that below _cvtsh_ss is turned off for MSVC?
There was a problem hiding this comment.
I wrote it wrong. It's only "intrin.h", no "x86intrin.h""
|
@szha now is only OSX.Maybe it should be changed to |
|
@yajiedesign yes, it's probably a clang problem. https://reviews.llvm.org/D16177 |
|
@piiswrong can this be merged ? |
msvc not inculde x86intrin.h and _cvtsh_ss