Ahh, the wonders of google. I believe I have found the problem kai's
change was originally intended to fix:
http://mingw-w64-public.narkive.com/jr5jMaJ2/quadmath-h-s-expq-crashes-at-runtime
And, I believe I have found what the actual problem was. I don't think
what kai did was the right fix. Let me explain why.
There is a big old structure full of obscure x87 floating point settings
defined as fenv_t in fenv.h (make sure you don't look at the ARM
definition by mistake). The format of this structure is defined by the
fldenv assembler instruction (so blame Intel).
Now, someone decided to be tricky and used some of the 'unused' data
members of this struct to store, not just the x87 FPU settings, but also
the SSE settings. It does this by overwriting the __unused0 and
__unused1 members of the struct.
So, when you call fegetenv(fenv_t * envp), what you actually get back is
a mixture of settings for both x87 and SSE. This isn't (normally) a
problem, since when you call fesetenv (const fenv_t * envp), it extracts
the SSE values back out of the struct, puts back the normal 'default'
values for __unused0 and __unused1, then calls fldenv to set the x87
values, followed by ldmxcsr for the SSE values.
However, quadmath doesn't use fegetenv to retrieve the current
settings. It uses feholdexcept. Unfortunately, feholdexcept doesn't
set __unused0 and __unused1 using the SSE flags the way fegetenv does.
As a result, when quadmath eventually calls fesetenv (&oldenv) to put
things back, the values in __unused0 and __unused1 are not valid SSE
flag settings. This is why quadmath was crashing.
Instead of fixing feholdexcept to use the same format that the rest of
the fenv_t functions use, kai changed fesetenv as described below.
I think the correct fix is something like the attached. This undoes
kai's change, and fixes feholdexcept instead. I've taken a brief look,
and I don't see any other functions that are like this.
Comments? Zidane, can you try this?
dw
On 12/19/2016 7:03 PM, David Wohlferd wrote:
FWIW, I agree that this code does not look right.
The purpose of fesetenv(const fenv_t * envp) is to change the settings
of the floating point environment as specified by the parameter.
However, instead of using the values from the parameter, the current
code *overwrites* the passed in settings with the current settings, then
sets using the result. Kinda defeats the purpose.
I hesitate to offer a fix, since I'm not sure what kai was trying to fix
with
https://sourceforge.net/p/mingw-w64/mingw-w64/ci/e98d8084dde3e3aba43736ee78dd7b35541df00b/
The description for his checkin ("Add a fix for working fesetenv in
libquadmath-library") is a little vague. Something relating to the sse
flags perhaps?
Kai, I don't suppose you remember any more details from this (> 3 year
old) checkin? It was a Monday, does that help?
dw
On 12/12/2016 2:26 AM, Zidane Sama wrote:
Fesetenv does not change fpu rounding mode. This bug already described
in https://sourceforge.net/p/mingw-w64/bugs/541/
diff --git a/mingw-w64-crt/misc/feholdexcept.c b/mingw-w64-crt/misc/feholdexcept.c
index a2b7834..b44c336 100644
--- a/mingw-w64-crt/misc/feholdexcept.c
+++ b/mingw-w64-crt/misc/feholdexcept.c
@@ -13,6 +13,7 @@
int feholdexcept (fenv_t * envp)
{
+ int ret = 0;
#if defined(_ARM_) || defined(__arm__)
fenv_t _env;
__asm__ volatile ("fmrx %0, FPSCR" : "=r" (_env));
@@ -20,10 +21,10 @@ int feholdexcept (fenv_t * envp)
_env.__cw &= ~(FE_ALL_EXCEPT);
__asm__ volatile ("fmxr FPSCR, %0" : : "r" (_env));
#else
- __asm__ __volatile__ ("fnstenv %0;" : "=m" (* envp)); /* save current into envp */
+ ret = fegetenv(envp);
/* fnstenv sets control word to non-stop for all exceptions, so all we
need to do is clear the exception flags. */
__asm__ __volatile__ ("fnclex");
#endif /* defined(_ARM_) || defined(__arm__) */
- return 0;
+ return ret;
}
diff --git a/mingw-w64-crt/misc/fesetenv.c b/mingw-w64-crt/misc/fesetenv.c
index d16dd65..015f71e 100644
--- a/mingw-w64-crt/misc/fesetenv.c
+++ b/mingw-w64-crt/misc/fesetenv.c
@@ -58,16 +58,14 @@ int fesetenv (const fenv_t * envp)
{
fenv_t env = *envp;
int _mxcsr;
- __asm__ ("fnstenv %0\n"
- "stmxcsr %1" : "=m" (*&env), "=m" (*&_mxcsr));
- /*_mxcsr = ((int)envp->__unused0 << 16) | (int)envp->__unused1; *//* mxcsr low and high */
+ _mxcsr = ((int)envp->__unused0 << 16) | (int)envp->__unused1; /* mxcsr low and high */
env.__unused0 = 0x;
env.__unused1 = 0x;
__asm__ volatile ("fldenv %0" : : "m" (env)
: "st", "st(1)", "st(2)", "st(3)", "st(4)",
"st(5)", "st(6)", "st(7)");
if (__mingw_has_sse ())
-__asm__ volatile ("ldmxcsr %0" : : "m" (*&_mxcsr));
+__asm__ volatile ("ldmxcsr %0" : : "m" (_mxcsr));
}
#endif /* defined(_ARM_) || defined(__arm__) */
--
Developer
