Bug 82626 - -msse and -mfpmath=sse Causes __FLT_EVAL_METHOD__ to be -1
Summary: -msse and -mfpmath=sse Causes __FLT_EVAL_METHOD__ to be -1
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 6.3.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-19 21:31 UTC by Alexander Grund
Modified: 2017-10-20 16:53 UTC (History)
2 users (show)

See Also:
Host:
Target: i?86-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Grund 2017-10-19 21:31:20 UTC
This was detected when compiling this:

echo "#include <cmath>" | g++ -x c++ - -msse -mfpmath=sse -o /dev/null

This combination of flags seems to cause a definition of __FLT_EVAL_METHOD__=-1 while it should be 2(?). This causes more issues later on code that relies on that define. See https://sourceforge.net/p/mingw/bugs/2352/
Comment 1 Andrew Pinski 2017-10-19 21:41:13 UTC
from: http://en.cppreference.com/w/c/types/limits/FLT_EVAL_METHOD

-1	the default precision is not known

See http://pubs.opengroup.org/onlinepubs/009695399/basedefs/float.h.html also
The use of evaluation formats is characterized by the implementation-defined value of FLT_EVAL_METHOD:

-1
Indeterminable.
Comment 2 jsm-csl@polyomino.org.uk 2017-10-19 21:54:02 UTC
I think a value of 0 should be correct with -mfpmath=sse.
Comment 3 Jakub Jelinek 2017-10-19 22:03:34 UTC
(In reply to joseph@codesourcery.com from comment #2)
> I think a value of 0 should be correct with -mfpmath=sse.

For -msse2 -mfpmath=sse sure, and that is what you get.
But for -mno-sse2 -msse -mfpmath=sse only float computations are done in SSE, double is done in x87.
Comment 4 Keith Marshall 2017-10-20 14:03:18 UTC
(In reply to Jakub Jelinek from comment #3)
> (In reply to joseph@codesourcery.com from comment #2)
> > I think a value of 0 should be correct with -mfpmath=sse.
> 
> For -msse2 -mfpmath=sse sure, and that is what you get.
> But for -mno-sse2 -msse -mfpmath=sse only float computations are done in
> SSE, double is done in x87.

If you're going to claim that argument, then your C++ <cmath> header is broken,
because you've just made the float_t and double_t data types indeterminate for -mfpmath=sse prior to SSE2, yet you unconditionally refer to both in "uses" clauses, (which requires typedefs from somewhere, yet you aren't providing them; if you expect me to provide them in MinGW <math.h> -- which is where Danny Smith originally provided them, but he neglected the "indeterminate" case -- then I'm probably going to make both equivalent to X87 defaults -- i.e. 80-bit long double for both -- and you might just as well punt float to the X87 too.  Thus, you've comprehensively broken support for pre-SSE2 -mfpmath=sse, from GCC-6 onwards).

Sorry, but you can't have your cake, and eat it; one way or another, this is a regression from GCC-5, which (correctly IMO, and in full agreement with Joseph) set FLT_EVAL_METHOD to zero, for the OP's use case.
Comment 5 Jakub Jelinek 2017-10-20 14:12:34 UTC
__FLT_EVAL_METHOD__ 0 can't be correct in that case, "all operations and constants evaluate in the range and precision of the type used." does not hold,
but "all operations and constants evaluate in the range and precision of long double." which is the requirement for 2 doesn't hold either - float operations evaluate in the range and precision of that type (float), double evaluates in the range and precision of long double.
float_t/double_t definitions aren't provided by GCC, but by the C library.
So, if they aren't defined properly in that case, it is up to glibc or mingw or whatever else projects owns them.
Comment 6 Keith Marshall 2017-10-20 15:24:41 UTC
(In reply to Jakub Jelinek from comment #5)
> __FLT_EVAL_METHOD__ 0 can't be correct in that case, "all operations and
> constants evaluate in the range and precision of the type used." does not
> hold,

__FLT_EVAL_METHOD__ -1 cannot be correct either: "the precision of floating point operations is indeterminable", so you force me to compromise on the typedefs for float_t and double_t, (and the only sane choice I can make is long double for both), yet you use methods of determinable precision for your implementation, (in terms of generated code, and in the case of which the double_t compromise is likely correct, but the float_t isn't).

> but "all operations and constants evaluate in the range and precision of
> long double." which is the requirement for 2 doesn't hold either - float
> operations evaluate in the range and precision of that type (float), double
> evaluates in the range and precision of long double.

Exactly so, (and there is no standardised FLT_EVAL_METHOD to represent that case), but it is very much a determinate case, ergo __FLT_EVAL_METHOD__ -1 is just plain wrong!  Correct would have been to exercise the licence granted by POSIX.1-2008, and to define an implementation specific value less than -1, whence I could then infer the proper (albeit non-standard combination of) type definitions for float_t and double_t.

> float_t/double_t definitions aren't provided by GCC, but by the C library.
> So, if they aren't defined properly in that case, it is up to glibc or mingw
> or whatever else projects owns them.

And therein lies your bug: if you've specified that float_t and double_t are of indeterminate precision, (and therefore -- by definition -- they themselves are undefinable), you have no right to gratuitously require their definitions in the C++ <cmath> header.
Comment 7 Jonathan Wakely 2017-10-20 15:45:50 UTC
I don't see any leeway in the C standard for an implementation to not define those types at all, but we can certainly make this change, if that's the right thing to do:

--- a/libstdc++-v3/include/c_global/cmath
+++ b/libstdc++-v3/include/c_global/cmath
@@ -1061,9 +1061,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #undef truncf
 #undef truncl
 
+#if 0 < __FLT_EVAL_METHOD__ && __FLT_EVAL_METHOD__ < 3
   // types
   using ::double_t;
   using ::float_t;
+#endif
 
   // functions
   using ::acosh;
Comment 8 Jakub Jelinek 2017-10-20 15:50:12 UTC
Indeed, this really is a mingw bug.
"and for other values of FLT_EVAL_METHOD, they are
otherwise implementation-defined."
Being implementation-defined doesn't mean they aren't defined at all.
Comment 9 Jonathan Wakely 2017-10-20 15:58:53 UTC
I doubt many people use those types at all in C++, so not defining them probably won't upset anybody. On the other hand, what we do today results in an unavoidable error.

This is probably a better way to do it:

--- a/libstdc++-v3/config/os/mingw32/os_defines.h
+++ b/libstdc++-v3/config/os/mingw32/os_defines.h
@@ -78,4 +78,9 @@
 // See libstdc++/59807
 #define _GTHREAD_USE_MUTEX_INIT_FUNC 1
 
+// See target/82626
+#if __FLT_EVAL_METHOD__ < 0 || __FLT_EVAL_METHOD__ > 2
+#define _GLIBCXX_NO_FLOAT_T_DOUBLE_T 1
+#endif
+
 #endif
--- a/libstdc++-v3/include/c_global/cmath
+++ b/libstdc++-v3/include/c_global/cmath
@@ -1061,9 +1061,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #undef truncf
 #undef truncl
 
+#ifndef _GLIBCXX_NO_FLOAT_T_DOUBLE_T
   // types
   using ::double_t;
   using ::float_t;
+#endif
 
   // functions
   using ::acosh;
Comment 10 Jakub Jelinek 2017-10-20 15:59:52 UTC
The footnote says:
The types float_t and double_t are intended to be the implementation’s most efficient types at least as wide as float and double, respectively. For FLT_EVAL_METHOD equal 0, 1, or 2, the type float_t is the narrowest type used by the implementation to evaluate floating expressions.
While defining float_t to float and double_t to long double for -msse -mfpmath=sse is a reasoanble choice, you need to have some choice also for other reasons why __FLT_EVAL_METHOD__ is -1 (e.g. mixed sse+387).
glibc just provides both float_t and double_t being long double for all the __FLT_EVAL_METHOD__ -1 cases.
Comment 11 Keith Marshall 2017-10-20 16:27:35 UTC
(In reply to Jakub Jelinek from comment #8)
> Indeed, this really is a mingw bug.

Wrong!  It is *both* a MinGW bug, *and* a GCC bug.  The difference is that I am fully committed to fixing the MinGW bug, whereas you seem to want to sweep the GCC bug under the carpet.

> "and for other values of FLT_EVAL_METHOD, they are
> otherwise implementation-defined."
> Being implementation-defined doesn't mean they aren't defined at all.

That's correct; (and it's also why simply suppressing references to float_t and double_t in <cmath> isn't the right thing to do).  However, neither is defining __FLT_EVAL_METHOD__ as "indeterminate" the right thing -- the case in question most definitely isn't indeterminate.

The right thing, in this case, is to exercise the licence granted by POSIX.1, (and by inference, by ISO-C), to specify an implementation-defined FLT_EVAL_METHOD, (POSIX.1-2008 stipulates that it should be less than -1), for which I can check in MinGW's <math.h>, and typedef float_t and double_t accordingly:

    :
  # elif __FLT_EVAL_METHOD__ == -2 /* say */
    typedef float float_t;
    typedef long double double_t;
    :
Comment 12 Keith Marshall 2017-10-20 16:45:03 UTC
(In reply to Jakub Jelinek from comment #10)
> While defining float_t to float and double_t to long double for -msse
> -mfpmath=sse is a reasoanble choice,

Actually, it is the correct choice, but...

> you need to have some choice also for other reasons why __FLT_EVAL_METHOD__ is 
> -1 (e.g. mixed sse+387).

...it would be a poor fit here, which is why FLT_EVAL_METHOD = -1 is completely the wrong configuration, in this case.  The correct solution is to introduce an implementation-defined FLT_EVAL_METHOD, (say -2), to accommodate this case, (since it cannot be represented by the standard 0..2 range of values, but neither is it, by any stretch of the imagination, an indeterminate case).

> glibc just provides both float_t and double_t being long double for all the
> __FLT_EVAL_METHOD__ -1 cases.

That's exactly what I propose to do, in MinGW; it is surely the most appropriate compromise for, e.g. sse+387.  However, -1 is just plain *wrong* for -msse.
Comment 13 jsm-csl@polyomino.org.uk 2017-10-20 16:53:53 UTC
Strictly, all x86 excess precision cases are indeterminable (semantics of 
-1) except for the case where -fexcess-precision=standard is used 
(requires front-end support, present for C only), because of the 
possibility of intermediate values getting narrowed when spilt to the 
stack; in the absence of -fexcess-precision=standard, the choice of 
FLT_EVAL_METHOD is a heuristic one based on what code is likely to expect 
values of that macro to mean (including based on 
-fpermitted-flt-eval-methods= to control whether TS 18661-3 values can be 
used).  Given the obscurity of the -msse -mno-sse2 -mfpmath=sse case I 
don't really think it makes sense to add a new enum flt_eval_method value 
and associated code that would be required to make that case of excess 
precision supported and predictable with -fexcess-precision=standard.