Use _mm_getcsr intrinsics on MSVC #1008
Conversation
test/evaluate/fp-testing.cpp
Outdated
| new_mxcsr |= 0x8000; | ||
| new_mxcsr |= 0x0040; | ||
| } else { | ||
| new_mxcsr &= 0x8000; |
There was a problem hiding this comment.
These are also broken, for the same reason as the ones above.
lib/evaluate/host.cpp
Outdated
| @@ -30,13 +33,25 @@ void HostFloatingPointEnvironment::SetUpHostFloatingPointEnvironment( | |||
| } | |||
| #if __x86_64__ | |||
| hasSubnormalFlushingHardwareControl_ = true; | |||
| #ifdef _MSC_VER | |||
| int new_mxcsr = _mm_getcsr(); | |||
There was a problem hiding this comment.
First, why is this necessary? Is fesetenv() broken on Windows?
Second, please conform to the surrounding style; specifically, we use modern braced initialization.
Third, you omitted the bitwise complementation of the right-hand sides of the &= forms, so they're not going to do what you think they're going to do.
There was a problem hiding this comment.
First, why is this necessary?
Because the fenv_t structure in windows doesn't have a __mxcsr. It looks like below,
typedef struct fenv_t
{
unsigned long _Fe_ctl, _Fe_stat;
} fenv_t;
Is fesetenv() broken on Windows?
No. It works fine.
There was a problem hiding this comment.
Why not #define __mxcsr _Fe_ctl for Windows and leave the rest of the code alone?
There was a problem hiding this comment.
I checked and that doesn't work.
There was a problem hiding this comment.
Here's the program that I ran,
#include <fenv.h>
#include <xmmintrin.h>
#include <stdio.h>
#ifdef _WIN32
#define __mxcsr _Fe_ctl
#endif
int main() {
fenv_t f;
fegetenv(&f);
unsigned int orig_mxcsr = _mm_getcsr();
printf("original value from _mm_getcsr %x\n", orig_mxcsr);
printf("original value from fgetenv %lx\n", f.__mxcsr);
f.__mxcsr |= 0x8040;
int ret = fesetenv(&f);
unsigned int new_mxcsr = _mm_getcsr();
printf("new value from _mm_getcsr %x\n", new_mxcsr);
}On Linux, this gave me,
$ ./a.out
original value from _mm_getcsr 1f80
original value from fgetenv 1f80
new value from _mm_getcsr 9fc0
On windows, this gave me,
original value from _mm_getcsr 1f80
original value from fgetenv 3f00003f
new value from _mm_getcsr 1f80
|
ping |
|
ping on this again. |
|
Superceded by https://reviews.llvm.org/D77815 |
No description provided.