[Bug libstdc++/50724] New: isnan broken by -ffinite-math-only in g++

[Bug libstdc++/50724] New: isnan broken by -ffinite-math-only in g++

[ejt at andrew dot cmu.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

             Bug #: 50724
           Summary: isnan broken by -ffinite-math-only in g++
    Classification: Unclassified
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: ejt@andrew.cmu.edu


Given the test code:

    static const float f_nan = 0.f/0.f;
    const int i_nan = *((int*)(&f_nan));
    printf("%g %d %x\n", f_nan, isnan(f_nan), i_nan);

I hope to see:
nan 1 7fc00000

but with the combination of -ffast-math and <cmath>, instead I get:
nan 0 7fc00000

Using just <math.h>, isnan will work. (hence I have filed against libstdc++) 
Not surprisingly, adding -fno-finite-math-only can also fix this.

(regarding cmath, both 'isnan' and 'std::isnan' give the same behavior; BTW I'm
a little surprised a unscoped 'isnan' compiles without 'using namespace
std;'... previous gcc (e.g. Apple 4.2.1) required the 'std'.  I see cmath
undef's the isnan macro from math.h, so where is the global isnan coming from?)

Tested with 4.4.3 and 4.5.2.

Ironically, a test with 4.1.2 shows the reverse situation: <math.h> didn't work
and <cmath> did work.

I realize the docs for -fno-finite-math-only basically warn "all bets are off"
for NaN handling, but as a quality of implementation issue, I'd like to see
consistent handling for floating-point classification functions.  A user may
not care about strict compliance for operations between non-finite values, but
if they are testing for 'bad' values (e.g. isnan), that's probably significant
-- otherwise the user wouldn't have added the classification calls to their
code in the first place.

Alternatively, a warning would be nice, like "isnan always evaluates to false"
when -ffinite-math-only is in effect.

[Bug libstdc++/50724] isnan broken by -ffinite-math-only in g++

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-14 09:15:35 UTC ---
With -ffinite-math-only you are telling that there are no NaNs and thus
GCC optimizes isnan (x) to 0.  That math.h "works" probably means it has
isnan implemented as macro expanding to inline assmbler.

[Bug libstdc++/50724] isnan broken by -ffinite-math-only in g++

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Ethan Tira-Thompson <ejtttje at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|INVALID                     |

--- Comment #2 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-14 14:27:13 UTC ---
Well, that's not actually true: -fno-finite-math is telling the compiler to
assume finite results during computation, but that doesn't mean there are "no
NaNs".  For example, I need isnan/isfinite to validate my users' input before I
can ensure that invariant afterward.  Clearly the bit pattern still exists in
the universe.

I'd also argue classification functions are not "arithmetic" and thus not
covered by -fno-finite-math ("Allow optimizations for floating-point
arithmetic")

Finally, I think you're being quick to brush off the consistency issue, both
with earlier gcc versions and with math.h.  I find it quite disconcerting that
isnan behavior changes between the two headers.

Is your concern that __builtin_isnan is called internally during fp
computations and we need to optimize that away in order to provide the speed
advantages requested by -fno-finite-math?  In that case, I propose cmath should
be updated to check __FINITE_MATH_ONLY__ in its user-facing isnan and fall back
to a safe method like math.h is currently using (apparently __isnan/__isnanf).

Otherwise, as I established above, I don't think it's safe to 'optimize' away
the floating point classification functions: the first issue above is code
safety/security, the second is pedantic regarding documentation, and the third
is a consistency issue.

Thanks!

[Bug libstdc++/50724] isnan broken by -ffinite-math-only in g++

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-14 14:31:33 UTC ---
(In reply to comment #2)
> Well, that's not actually true: -fno-finite-math is telling the compiler to
> assume finite results during computation

No, it tells the compiler that all inputs are finite.

[Bug libstdc++/50724] isnan broken by -ffinite-math-only in g++

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-10-14 14:47:18 UTC ---
(In reply to comment #0)
> if they are testing for 'bad' values (e.g. isnan), that's probably significant
> -- otherwise the user wouldn't have added the classification calls to their
> code in the first place.

if they are compiling with -ffinite-math-only (i.e. asserting no NaN arguments
or results), that's probably significant -- otherwise the user wouldn't have
added the switch to their compilation command in the first place.

[Bug libstdc++/50724] isnan broken by -ffinite-math-only in g++

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #5 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-14 14:56:27 UTC ---
I haven't really followed this discussion, but I remember a very similar one
some time ago, and I suspect that part of the confusion stems from the meaning
of "finite-math": maybe some users don't consider calling std::is_nan part of a
*mathematical computation* proper, they consider it something preliminary to
it, they would like to see *only* arithmetic expressions, transcendental
functions calls, etc, optimized per finite-math. I don't know if I have been
able to explain myself well enough. Anyway, mine is just a side observation,
maybe worth keeping in mind only when writing the documentation.

[Bug libstdc++/50724] isnan broken by -ffinite-math-only in g++

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #6 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-14 15:07:40 UTC ---
I was also thinking, maybe from the user point of view, a good way to deal with
this kind of problem today is splitting the computation in parts via the new
optimization attribute and pragma, keep the is_nan & co classifications outside
the -fast-math cores.

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[matz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Michael Matz <matz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
   Last reconfirmed|                            |2011-10-14
                 CC|                            |matz at gcc dot gnu.org
          Component|libstdc++                   |middle-end
         Resolution|INVALID                     |
     Ever Confirmed|0                           |1
           Severity|normal                      |enhancement

--- Comment #7 from Michael Matz <matz at gcc dot gnu.org> 2011-10-14 15:36:26 UTC ---
Hmm.  I do sympathise with Ethan.  I never was very convinced that
__builtin_isnan should return 0 even for NaN bit patterns just because of
-ffast-math.  After all we don't gain much by this in terms of optimization
opportunities (we don't emit calls to it from inside GCC).

And Ethan is right that a developer might indeed first test values coming from
outside the program for fulfilling the guarantees the program is expecting.
After all, -ffinite-math-only is an assertion by the compiler user, but
in light of user input he has to first establish these assertions before
using code paths that need those assertions.  isnan() is precisely the way
to establish them.

And no, I'm not convinced about the argument that the program author simply
shouldn't have used -ffinite-math-only.  He can't control user input, or
better said, the way he can control it is by the classification functions.
Hence we shouldn't remove this (only) way for him to do the classification.

Note how I wrote classification.  IMO the same applies to isnan, isinf,
isnormal, isfinite and fpclassify.

Note further that in this light I don't necessarily think that the precise
testcase from the initial comment as written there should work.  There the
NaN isn't coming from user input but is program generated, and for _those_
values the assertion of -ffinite-math-only should already hold.  Not that
I see any reason to differ between the cases.

Reopening as enhancement request as it would be a change in documented
behaviour.

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-10-14 16:03:06 UTC ---
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25975
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28795
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28796

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #9 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-14 20:07:41 UTC ---
Thanks, I think Michael hit the nail on the head for summarizing my intention,
I'm satisfied to file this as a feature request (although personally I'd still
call it a bug ;))

For reference regarding the test case, I agree with your point that 0.f/0
needn't be a reliable NaN generator under -ffast-math.  The original source was
actually using std::numeric_limits<float>::quiet_NaN() and/or signaling_NaN(),
but I switched to 0.f/0 in order to test under C (not-++) and forgot to switch
back.  So yeah, considering numeric_limits still returns the NaN bit patterns
regardless of -ff-m-o, it's unbalanced that isnan is prevented from detecting
them.  Regardless, disabling classification routines is going beyond the scope
of "math" and "arithmetic" as per the name of the flag and its documentation.

(For those oversimplifying -ff-m-o as "No NaNs", perhaps you would like to add
a new -fno-nans or perhaps -fno-quiet-nans flag if that is what you really want
-ffinite-math-only to mean...)

In the three linked bug reports, the first is basically a "don't do that"
without discussion, the second specifically complains about the lack of
discussion, and the third is redirected toward fixing an issue in -mno-ieee
without resolving the -ffinite-math-only aspect.

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #10 from Marc Glisse <marc.glisse at normalesup dot org> 2011-10-14 21:12:40 UTC ---
As a library writer, having isnan return false is precisely what I am expecting
from -ffinite-math-only. In my code, I implement regular computations for
finite numbers, and I need some special cases for infinite and nan values,
which I test with isnan and isfinite. With -ffinite-math-only, the compiler is
able to remove those special cases entirely.

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #11 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-14 22:07:44 UTC ---
Marc: is this code perusable? I'm curious because I expect either the
calculations may generate NaN or not at all.  If they might and you even have
test cases to handle it, then I'm surprised you would ever want to support
running with -ff-m-o.  Conversely if you knew the code doesn't generate the
nonfinite values, then you don't need the classifications in the first
place...?

I'm guessing (and apologies if this is inaccurate) that this might boil down to
saying that you want to interpret an end user setting -ff-m-o as an opportunity
to skip validating their input or skip doing assertions during its processing,
which could be a reasonable thing to do, but that's a choice I'd rather leave
to individual developers, e.g. can also wrap code with #if
__FINITE_MATH_ONLY__==0 or such...

Or in other words, it's only a missed optimization if you wind up with
classification calls, whereas it's a full-fledged execution error when NaN gets
past validation.

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |INVALID

--- Comment #12 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-15 08:50:56 UTC ---
(In reply to comment #11)
> Marc: is this code perusable? I'm curious because I expect either the
> calculations may generate NaN or not at all.  If they might and you even have
> test cases to handle it, then I'm surprised you would ever want to support
> running with -ff-m-o.  Conversely if you knew the code doesn't generate the
> nonfinite values, then you don't need the classifications in the first
> place...?
> 
> I'm guessing (and apologies if this is inaccurate) that this might boil down to
> saying that you want to interpret an end user setting -ff-m-o as an opportunity
> to skip validating their input or skip doing assertions during its processing,
> which could be a reasonable thing to do, but that's a choice I'd rather leave
> to individual developers, e.g. can also wrap code with #if
> __FINITE_MATH_ONLY__==0 or such...
> 
> Or in other words, it's only a missed optimization if you wind up with
> classification calls, whereas it's a full-fledged execution error when NaN gets
> past validation.

You can switch between explicit checking and trapping for example, by
switching between -ffinite-math-only and -fno-trapping-math.

Note that in general it is impossible to decide whether an argument of
isnan() is from user input or previous computation.  Which means that
making isnan() special for -ffinite-math-only makes as much sense as
special-casing any math library function (that may also take user input).

This has been discussed to death already, and the present behavior is how
GCC behaved since ever.  It's not a bug.  The documentation states

"Allow optimizations for floating-point arithmetic that assume
that arguments and results are not NaNs or +-Infs."

it is clear that isnan(x) may then be optimized assuming that x is
not NaN.

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #13 from Marc Glisse <marc.glisse at normalesup dot org> 2011-10-15 09:05:57 UTC ---
(In reply to comment #11)
> I'm guessing (and apologies if this is inaccurate) that this might boil down to
> saying that you want to interpret an end user setting -ff-m-o as an opportunity
> to skip validating their input or skip doing assertions during its processing,

Yes, exactly.

> which could be a reasonable thing to do, but that's a choice I'd rather leave
> to individual developers, e.g. can also wrap code with #if
> __FINITE_MATH_ONLY__==0 or such...

Not very portable afaik.

> Or in other words, it's only a missed optimization if you wind up with
> classification calls,

-ffinite-math-only exists for the sole purpose of allowing extra optimizations
at the expanse of correctness if the contract is broken.

> whereas it's a full-fledged execution error when NaN gets
> past validation.

A full-fledged user error to have used this flag ;-)

Actually, I can understand why you'ld want to be able to write
assert(!isnan(x)) and get a useful result. If on the other hand you want
if(isnan(x))f();else g(); then g should be in a separate translation unit that
can use -ffinite-math-only (and using LTO may require a lot of prayers in that
case) or it could use the optimize attribute/pragma. This could work for the
assertion as well: have a validate_input function compiled without the flag.

I am not sure if it is possible to have it both ways: be able to validate the
input and still be able to optimize headers.

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Ethan Tira-Thompson <ejtttje at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |

--- Comment #14 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-15 14:01:11 UTC ---
Richard said:
> The documentation states
> "Allow optimizations for floating-point arithmetic that
> assume that arguments and results are not NaNs or +-Infs."

Yes, that's my argument as well.  Note ARITHMETIC.  Also note MATH in the name
of both --finite-MATH-only, and -ffast-MATH it falls under.  And it doesn't
reference the 'math library' to qualify that.

Basically, if you want to close on this point, I want to see you explicitly
argue why the classification functions should be considered arithmetic.

I'm going to nail this down and list there are 5 classification functions
(fpclassify, infinite, isinf, isnan, isnormal) and the vast majority of the
other library functions are obviously proper arithmetic operations.  The ones
that aren't (signbit, copysign, nextafter, nan), you're exactly right that we
should carefully consider the result of NaN optimization ("special cases"). 
You don't have to do this if *you* don't want to, but it should be done and it
sounds like no one has.

> This has been discussed to death already, and the present behavior is how
> GCC behaved since ever.

Except also NOT TRUE.  It currently doesn't behave this way with math.h.  It
didn't behave this way in 4.1 (Fedora) or 4.2 (Apple).  I only got screwed by
this by the CHANGE in behavior on upgrading to 4.4.  (Not sure about 4.3).  I
already presented this in the original post at the top (except the Apple test
is a new data point; FYI Apple gcc math.h also 'works', so either 4.2 was
generally consistent or Apple likes to patch theirs for consistency)

This is further evidence gcc has not had a good policy discussion of where NaN
optimizations should be applied, because the implementation keeps changing, and
it's not even consistent between math.h and cmath within any given version
other than 4.2-Apple.

Marc said:
>> __FINITE_MATH_ONLY__
> Not very portable afaik.

Neither is -ffast-math, add a 'defined(__FINITE_MATH_ONLY__) &&' and it will be
applied opportunistically when available, or even better: the user can
-D__FINITE_MATH_ONLY__=1 themselves on non-gcc platforms and still get the
performance benefit you're looking for even without -ffast-math support, so
it's a double win.  IMHO, on the other hand it's harder and more error prone to
override isnan with my own implementation.

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |INVALID

--- Comment #15 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-10-15 18:00:10 UTC ---
It has always been assumed that arithmetic includes isnan.  isnan can be
implemented as a macro defining to !(a==a) which then becomes an arithmetic.

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #16 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-16 09:59:35 UTC ---
(In reply to comment #14)
> Richard said:
> > The documentation states
> > "Allow optimizations for floating-point arithmetic that
> > assume that arguments and results are not NaNs or +-Infs."
> 
> Yes, that's my argument as well.  Note ARITHMETIC.  Also note MATH in the name
> of both --finite-MATH-only, and -ffast-MATH it falls under.  And it doesn't
> reference the 'math library' to qualify that.
> 
> Basically, if you want to close on this point, I want to see you explicitly
> argue why the classification functions should be considered arithmetic.
> 
> I'm going to nail this down and list there are 5 classification functions
> (fpclassify, infinite, isinf, isnan, isnormal) and the vast majority of the
> other library functions are obviously proper arithmetic operations.  The ones
> that aren't (signbit, copysign, nextafter, nan), you're exactly right that we
> should carefully consider the result of NaN optimization ("special cases"). 
> You don't have to do this if *you* don't want to, but it should be done and it
> sounds like no one has.
> 
> > This has been discussed to death already, and the present behavior is how
> > GCC behaved since ever.
> 
> Except also NOT TRUE.  It currently doesn't behave this way with math.h.

math.h is not part of GCC, if math.h implements isnan in a way that skips
GCC (by making it use inline assembler) then that doesn't show GCC does
not perform the optimization (you can check with using __builtin_isnan
instead of isnan).

The only way out I see that not breaks other users uses would be a new
flag, like -fpreserve-ieee-fp-classification that, ontop of -ffinite-math-only,
would disable the optimization for the classification functions.
(Note that they are folded to arithmetic, !(x==x), so that transform has
to be disabled as well, and on some architectures you might get library
calls because of this instead of inline expansions).

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Ethan Tira-Thompson <ejtttje at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |

--- Comment #17 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-17 04:12:31 UTC ---
Richard said:
> math.h is not part of GCC

But the point is there is value in consistency between math.h and cmath
regardless of who owns math.h.  I'm asserting that this value is greater than
that gained by 'optimizing away' the classification functions in cmath. 
Inconsistency leads to confused users and therefore bugs, missed optimization
is only going to cause slower code.  I get that you want to make the most of
-ffast-math, and if it were a big speedup it could be worthwhile, but it seems
reasonable that if someone is serious about optimizing away their
classification sanity checks in a release build, they would be better served by
using assert() or an #ifdef instead of relying of the vagaries of -ffast-math
for this purpose.

> The only way out I see that not breaks other users uses would be a new
> flag, like -fpreserve-ieee-fp-classification that, ontop of -ffinite-math-only,

I'm not opposed to a new flag, but I'd suggest the reverse semantics. 
Disabling classification is an extra degree of non-compliance beyond ignoring
non-finite math operations.  I'd rather users add flags to progressively become
less compliant, rather than add a flag to get some compliance back.

But to rewind a second, I want to address the "breaks other users" comment...
here is the status AFAIK:
1) Older versions (4.1, 4.2) of gcc did not do this optimization of
classification functions.  Thus, "legacy" code expects classification to work
even in the face of -ffast-math, which was changed circa 4.3/4.4
2) Removing classification 'breaks' code because it fundamentally strips
execution paths which may otherwise be used.
3) Leaving classification in could be considered a missed optimization, but is
at worst only a performance penalty, not a change in execution values.
4) Personal conjecture: I doubt the classification routines are a performance
bottleneck in the areas where -ff-m-o is being applied, so (3) is pretty
minimal.  And I seriously doubt anyone is relying on the removal of
classification in a code-correctness context, that doesn't make any sense.

Are we on the same page with these points?  So if you are concerned with the
breakage of existing code, isn't the solution to revert to the previous scope
of the -ff-m-o optimization ASAP, and then if there is a desire to extend the
finite-only optimization to classification functions, *that* would be a new
feature request, perhaps with its own flag?

> (Note that they are folded to arithmetic, !(x==x), so that transform
> has to be disabled as well, and on some architectures you might get
> library calls because of this instead of inline expansions).

I'd rather leave comparison optimizations as they are under -ff-m-o, that seems
a sensible expectation of the 'arithmetic' scope, and is relatively well-known,
long-standing effect of -ffast-math.  It's only the 5 explicit fp
classification calls which I think deserve protection to allow data validation
in a non-hacky manner before doing core computations with the finite invariant.

Unless you are saying things like std::isnan cannot be implemented separately
from !(x==x)?  That has not been my understanding.

[Bug middle-end/50724] isnan broken by -ffinite-math-only in g++

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |DUPLICATE

--- Comment #18 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-17 07:12:38 UTC ---
(In reply to comment #17)
> Richard said:
> > math.h is not part of GCC
> 
> But the point is there is value in consistency between math.h and cmath
> regardless of who owns math.h.  I'm asserting that this value is greater than
> that gained by 'optimizing away' the classification functions in cmath. 
> Inconsistency leads to confused users and therefore bugs, missed optimization
> is only going to cause slower code.

You get the same inconsistency if math.h implements isnan as

#define isnan(x) (!((x)==(x)))

which is a valid implementation.  That would be optimized with your
suggested -ffinite-math-only implementation, but not when the library
implements isnan as

#define isnan(x) __builtin_isnan(x)

So what's your point again?

> I get that you want to make the most of
> -ffast-math, and if it were a big speedup it could be worthwhile, but it seems
> reasonable that if someone is serious about optimizing away their
> classification sanity checks in a release build, they would be better served by
> using assert() or an #ifdef instead of relying of the vagaries of -ffast-math
> for this purpose.
> 
> > The only way out I see that not breaks other users uses would be a new
> > flag, like -fpreserve-ieee-fp-classification that, ontop of -ffinite-math-only,
> 
> I'm not opposed to a new flag, but I'd suggest the reverse semantics. 
> Disabling classification is an extra degree of non-compliance beyond ignoring
> non-finite math operations.  I'd rather users add flags to progressively become
> less compliant, rather than add a flag to get some compliance back.

The point is backward-compatible behavior of -ffast-math.  We can't and
should not break this without a very very very good reason.  Which this isn't.

> But to rewind a second, I want to address the "breaks other users" comment...
> here is the status AFAIK:
> 1) Older versions (4.1, 4.2) of gcc did not do this optimization of
> classification functions.  Thus, "legacy" code expects classification to work
> even in the face of -ffast-math, which was changed circa 4.3/4.4

Sure they did.

> 2) Removing classification 'breaks' code because it fundamentally strips
> execution paths which may otherwise be used.
> 3) Leaving classification in could be considered a missed optimization, but is
> at worst only a performance penalty, not a change in execution values.
> 4) Personal conjecture: I doubt the classification routines are a performance
> bottleneck in the areas where -ff-m-o is being applied, so (3) is pretty
> minimal.  And I seriously doubt anyone is relying on the removal of
> classification in a code-correctness context, that doesn't make any sense.

I have written code that you can switch between using FP exceptions and
explicit checks at certain points.  I expect the checks to be optimized
away when using the FP exception path.

> Are we on the same page with these points?  So if you are concerned with the
> breakage of existing code, isn't the solution to revert to the previous scope
> of the -ff-m-o optimization ASAP, and then if there is a desire to extend the
> finite-only optimization to classification functions, *that* would be a new
> feature request, perhaps with its own flag?
> 
> > (Note that they are folded to arithmetic, !(x==x), so that transform
> > has to be disabled as well, and on some architectures you might get
> > library calls because of this instead of inline expansions).
> 
> I'd rather leave comparison optimizations as they are under -ff-m-o, that seems
> a sensible expectation of the 'arithmetic' scope, and is relatively well-known,
> long-standing effect of -ffast-math.  It's only the 5 explicit fp
> classification calls which I think deserve protection to allow data validation
> in a non-hacky manner before doing core computations with the finite invariant.
> 
> Unless you are saying things like std::isnan cannot be implemented separately
> from !(x==x)?  That has not been my understanding.

No, I said that GCC itself unconditionally transforms isnan to !(x == x)
(independent of -ffinite-math-only).

If you really want to go forward you have to produce a patch, test it and
post it to gcc-patches@gcc.gnu.org.

*** This bug has been marked as a duplicate of bug 25975 ***

[Bug libstdc++/50724] cmath's floating-point classification implementations inconsistent with math.h

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Ethan Tira-Thompson <ejtttje at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
          Component|middle-end                  |libstdc++
         Resolution|DUPLICATE                   |
            Summary|isnan broken by             |cmath's floating-point
                   |-ffinite-math-only in g++   |classification
                   |                            |implementations
                   |                            |inconsistent with math.h
           Severity|enhancement                 |normal

--- Comment #19 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-17 15:31:12 UTC ---
Richard said:
>> 1) Older versions (4.1, 4.2) of gcc did not do this optimization
> Sure they did.

Dude, I tested this in my very first post.  I'm only here because we had
working code which has broken on the upgrade to Ubuntu 10.04.  There's no point
in discussing with you if you're just going to deny the state of the world and
not offer any data to back up your side.  But before you start coding a test
case, read on, turns out I've already done this for you below.

> I expect the checks to be optimized away when using the FP exception path.

I hate using this rationale, but have you considered that this is not a
required or portable behavior and you shouldn't rely on it?  And what happens
if the checks are left in?  Is anything actually 'broken' by this?  You keep
using that word, I do not think it means what you think it means.  I lose data
validation when the classifications are disabled.  My code does something
fundamentally different as a result.  This is demonstrable 'breakage'.  To
quote a wise man, "We should not break this without a very very very good
reason.  Which this isn't." ;)

> [isnan implementation ...] So what's your point again?

There are various ways to define isnan and friends.  I'm requesting one which
is consistent with math.h's isnan and also previous behavior.  That could be
bitmask operations, it could be calling __isnan or math.h's isnan(), etc. 

I think we need some new data to move this along...
I did a little investigation on how these functions have been defined over
time:

In 4.1 and 4.2, cmath provides:
  std::isnan() forwards to __gnu_cxx::__capture_isnan()
  __gnu_cxx::__capture_isnan() forwards to math.h ::isnan()
  math.h ::isnan forwards to __isnan()
as of this patch:
http://gcc.gnu.org/viewcvs/trunk/libstdc%2B%2B-v3/include/c_std/cmath?r1=119611&r2=130443
This has changed to simply:
  std::isnan() forwards to __builtin_isnan()

Huh, that's interesting, let's cut out the middle men and see what these
underlying functions do across history:
  const float qNaN = std::numeric_limits<float>::quiet_NaN();
  std::cout << __builtin_isnan(qNaN) << ' '
            << __isnan(qNaN) << ' '
            << !(qNaN==qNaN) << '\n';

Compiled with -ffast-math on 3 different machines
0 1 0 // Fedora 9, gcc 4.1.2
0 1 0 // Apple 10.7, gcc 4.2.1
0 1 0 // Ubuntu 10.04, gcc 4.4.3

Hey, you're right insofar as the 'internal' implementations haven't changed at
all.  What changed is where cmath sends its implementation (and fyi, I did
originally file this under libstdc++, just by lucky guess ;).  cmath used to
explicitly call the math.h definition (aka __isnan), which is not optimized by
-ffast-math.

For reference, I checked the headers on the Apple machine as well, and found
some relevant results.  cmath starts the same with std::isnan → __capture_isnan
→ ::isnan, but the abridged math.h ::isnan definition is:
  #if defined( __GNUC__ ) && 0 == __FINITE_MATH_ONLY__ 
    #define isnan(x) __inline_isnanT((T)(x))
    inline int __inline_isnanT( T __x ) { return __x != __x; }
  #else
    #define isnan(x) __isnanT((T)(x))
    extern int __isnanT(T);
  #endif
(full source
http://www.opensource.apple.com/source/Libm/Libm-315/Source/Intel/math.h)

Which is all prepended by this comment:
> Yes, that's right. You only get the fast iswhatever() macros if you do NOT
> turn on -ffast-math.  These inline functions require the compiler to be
> compiling to standard in order to work. -ffast-math, among other things,
> implies that NaNs don't happen. The compiler can in that case optimize
> x != x to be false always, wheras it would be true for NaNs. That breaks
> __inline_isnan() below.

So, to whatever degree you care what major users are doing, at least one
popular platform considers it breakage to disable fp classification, and falls
back on a function call to preserve it in the face of -ffast-math.

> The point is backward-compatible behavior of -ffast-math.

I agree!  And I even found the exact patch that broke it.  So would anyone (Hi
Paolo :)) like to chime in on the rationale of the linked patch above and how
best to restore consistency of the user-facing classification functions?

In particular, can std::isnan (and its classification friends) be redirected
back to their math.h implementation?

Or if there's a performance hit with using math.h, could they be wrapped in an
#ifdef similar to what Apple did, so that you use __builtin_isnan normally, but
capture to math.h when __FINITE_MATH_ONLY__ is detected?

Thanks!

[Bug libstdc++/50724] cmath's floating-point classification implementations inconsistent with math.h

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #20 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-17 15:40:52 UTC ---
No way the library is going to do anything else but forward to the builtins
here.

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
          Component|libstdc++                   |middle-end
         Resolution|                            |DUPLICATE

--- Comment #21 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-17 15:43:57 UTC ---
.

*** This bug has been marked as a duplicate of bug 25975 ***

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Ethan Tira-Thompson <ejtttje at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|DUPLICATE                   |

--- Comment #22 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-17 16:46:27 UTC ---
Paolo: thanks for the quick reply, but it would help if you could explain why
that is the case?  Also, a follow-up, is __builtin_isnan and friends used
anywhere except cmath?  It appears not, other than tr1/special_function_util.h,
which is also providing an __isnan.

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |DUPLICATE

--- Comment #23 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-17 16:49:03 UTC ---
.

*** This bug has been marked as a duplicate of bug 25975 ***

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Ethan Tira-Thompson <ejtttje at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|DUPLICATE                   |

--- Comment #24 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-17 18:38:05 UTC ---
Please don't change bug status without comment.  We already have bug 25975
listed as a duplicate in the history several times now.  That bug report was
never really resolved, the user just found a workaround and went away. 
Further, he was using math.h, which has been stated is outside gcc, whereas I
am addressing the cmath implementation.  So not even the same issue anyway.

So that we can find a resolution to this issue, please explain why cmath cannot
use math.h?  I'm assuming there's a good reason, I just don't know the history
there.  Is it performance, portability, licensing, what? (Those who don't
understand history (me) are doomed to repeat it...)

Alternatively, another potential solution would be to tweak the __builtins to
restore the original behavior in cmath, which is what you (Paolo) were actually
suggesting early on... grepping around, it certainly looks like cmath is the
only thing using them.  Am I missing anything there?

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
                 CC|daniel.kruegler at          |
                   |googlemail dot com          |
         Resolution|                            |DUPLICATE

--- Comment #25 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-10-17 18:53:56 UTC ---
.

*** This bug has been marked as a duplicate of bug 25975 ***

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Ethan Tira-Thompson <ejtttje at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|DUPLICATE                   |

--- Comment #26 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-17 19:21:56 UTC ---
I'm just asking about a patch that broke existing code and this is how you
respond?  It's nothing personal, mistakes happen, I'm just trying to figure out
how to fix it for others.  (And why'd you remove someone else's name off the cc
list?  Whatever...)  If you don't have time to support your code then just say
so or ignore it, no need to be an ass about it.

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |DUPLICATE

--- Comment #27 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-10-17 19:25:06 UTC ---
The patch just made cmath use the builtins and the builtin isnan is implemented
as !(a==a) which gets optimized to false because with only finite math, a has
to equal itself.

*** This bug has been marked as a duplicate of bug 25975 ***

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Ethan Tira-Thompson <ejtttje at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|DUPLICATE                   |

--- Comment #28 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-17 20:21:27 UTC ---
So then there are a variety of potential solutions to evaluate:

A) Don't use the builtin, go back to __isnan, perhaps only when -ff-m-o is in
effect.  Paolo says this is bad, but it's not clear why.  Seems like a decent
solution except for his apparent disapproval.

B) Change the builtin implementation, just for example with bitmasks:
  inline bool isnan(float x) {
    const int EXP  = 0x7f800000;
    const int FRAC = 0x007fffff;
    const int y = *((int*)(&x));
    return ((y&EXP)==EXP && (y&FRAC)!=0);
  }
(this is what I'm using to reproduce isnan in my own code, maybe there are
better ways?)
But this may be a portability issue to support different float point standards,
right?  That's just as much an argument to not have users trying to do this
themselves though.

C) This is getting out of my knowledge, but Paolo also suggested "splitting the
computation in parts via the new optimization attribute and pragma, keep the
is_nan & co classifications outside the -fast-math cores."  That sounds elegant
and possibly straightforward if it's just a matter of adding an attribute.

D) Combination of A, B and/or C: add new 'safe' builtins, have cmath use them
when -ff-m-o is in effect, but otherwise use the presumably faster/more easily
optimized normal builtins when these optimizations won't change behavior.

E) Screw compatibility with older gcc or Apple's current gcc or other forks,
just update the documentation.  Make it clear -ff-m-o includes both
classification and arithmetic functions, that this behavior is not portable or
consistent even within math.h vs. cmath, and let the users suck it up. 
Generating warnings on calling the nan-generation functions like nan(char*) or
numeric_limits::quiet_NaN() and maybe the classification functions too (e.g.
"isX always evaluates to true/false") would be a plus.

I'll be pretty disappointed with gcc quality control if you really choose (E),
but it's there.

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |DUPLICATE

--- Comment #29 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-10-17 21:05:02 UTC ---
I only see one person being an ass here. I'll update the documentation to make
it clear, just stop reopening this.

*** This bug has been marked as a duplicate of bug 25975 ***

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #30 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-10-17 21:21:45 UTC ---
(In reply to comment #28)
> So then there are a variety of potential solutions to evaluate:
> 
> A) Don't use the builtin, go back to __isnan, perhaps only when -ff-m-o is in
> effect.  Paolo says this is bad, but it's not clear why.  Seems like a decent
> solution except for his apparent disapproval.

Maybe you should consider that the maintainers have actually thought about it
and might know what they're talking about?  What is __isnan? Is that reliably
available on all supported platforms?  (Hint: no)  Is __builtin_isnan reliably
available? (Hint: yes)

Being patronising because you don't understand all the issues won't win you any
friends.

> B) Change the builtin implementation, just for example with bitmasks:
>   inline bool isnan(float x) {
>     const int EXP  = 0x7f800000;
>     const int FRAC = 0x007fffff;
>     const int y = *((int*)(&x));
>     return ((y&EXP)==EXP && (y&FRAC)!=0);
>   }
> (this is what I'm using to reproduce isnan in my own code, maybe there are
> better ways?)
> But this may be a portability issue to support different float point standards,
> right?  That's just as much an argument to not have users trying to do this
> themselves though.

If you want your code to conform to floating point standards THEN DON'T USE
-ffinite-math-only! It's there in the manual for all to see.

> C) This is getting out of my knowledge, but Paolo also suggested "splitting the
> computation in parts via the new optimization attribute and pragma, keep the
> is_nan & co classifications outside the -fast-math cores."  That sounds elegant
> and possibly straightforward if it's just a matter of adding an attribute.

That would be something for user code to do, not the library.
Feel free to do it in your code.

> D) Combination of A, B and/or C: add new 'safe' builtins, have cmath use them
> when -ff-m-o is in effect, but otherwise use the presumably faster/more easily
> optimized normal builtins when these optimizations won't change behavior.
> 
> E) Screw compatibility with older gcc or Apple's current gcc or other forks,
> just update the documentation.  Make it clear -ff-m-o includes both
> classification and arithmetic functions, that this behavior is not portable or
> consistent even within math.h vs. cmath, and let the users suck it up. 
> Generating warnings on calling the nan-generation functions like nan(char*) or
> numeric_limits::quiet_NaN() and maybe the classification functions too (e.g.
> "isX always evaluates to true/false") would be a plus.

Feel free to open a separate PR requesting a warning.

> I'll be pretty disappointed with gcc quality control if you really choose (E),
> but it's there.

I think we can live with your disappointment.

You are using a feature which is documented to alter behaviour for code using
NaNs, it clearly says "it can result in incorrect output for programs which
depend on an exact implementation of IEEE or ISO rules/specifications for math
functions".  If you don't like it, don't use the option.

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #31 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-18 00:33:41 UTC ---
I don't see what the hurry is to close the bug while it's still under
discussion.  I guess you guys just like hit that 'resolved' button before
you've actually committed any resolution.  OK, when in Rome...

> Maybe you should consider that the maintainers have actually thought about
> it and might know what they're talking about?  What is __isnan? Is that
> reliably available on all supported platforms?  (Hint: no)  Is __builtin_isnan
> reliably available? (Hint: yes)

This was exactly the information I was asking for.  I explicitly asked if it
was a portability issue (vs. performance or licensing, thought perhaps maybe it
was some GPL thing).  Maybe it sounded patronizing because it was such a
obvious question to you, but please have a little patience with people who are
new to the project internals.  Heck, how many people do you think even know
that __isnan is from libm and __builtin_isnan is from gcc, or the relationship
between the two?  This stuff is not well documented nor common knowledge, and
if maintainers don't want to answer questions then what do you expect?

A fair bit of this thread has been wasted on a subset of maintainers saying
'gcc hasn't changed, finite-math has always been this way', directly in
response to me showing code demonstrating the change and eventually tracking
down the patch that caused it in spite of the derogatory comments, like even
yours just now.  So similarly you might consider a user who has spent a fair
bit of time finding the problem and presenting it to you on a silver platter,
is probably not hallucinating the whole thing. (Hint: no)  Fair enough?

This is probably moot, but if you'd like to get this back on topic, I'm still
on board.

So then, just for reference, does no part of gcc rely on libm now?  Or is there
a config mechanism that lets parts of gcc use libm when it is available?  I ask
because gcc (well, libstdc++) used to use libm, so that begs the question what
is the current status?

> If you want your code to conform to floating point standards THEN DON'T
> USE -ffinite-math-only! It's there in the manual for all to see.

My concern is not 'conformance' per se.  Obviously then I just wouldn't be
using the flag.

1) Inconsistency between math.h and cmath (unpredictable optimization
application, maybe I use math.h, but someone includes cmath before my header,
perhaps a precompiled header across translation units, now my code behaves
differently, and this might vary per-translation unit within the program.)
2) Inconsistency with older versions/forks of gcc (breaking legacy
code/portability, maybe even security issues?)
3) Insufficient documentation regarding the expected degree of non-compliance
("arithmetic" includes classification?)
4) What is really the *desired* degree of non-compliance in practical usage, is
this throwing out the baby with the bathwater?  (For most of the finite-math
effects, garbage-in-garbage-out is fair enough; but the classification
functions often lead to code paths with side effects, e.g. triggering UI,
skipping data records, etc, so it's a much broader effect.)

So yes, I could just not use the flag.  But it's useful, it's just gotten too
aggressive.  As a user, it's hard to reproduce isnan (and friends) to do
validation when finite-math is active (and since build systems make it easy for
end users to recompile with additional flags, it's not entirely controllable
either, other than saying "don't do that", I'd rather it wasn't
all-or-nothing.)  Since gcc used to provide this more limited finite-math
optimization, I was hoping it would not be hard to restore it.  I also used to
think there would be a concern regarding how the user-visible scope of
finite-math had changed, breaking code. *shrug*

> That would be something for user code to do, not the library.
> Feel free to do it in your code.

I can't add an attribute to the system isnan from my user code, or can I?  I've
never been quite sure what Paolo was referring to, can someone clue me in?  It
sounded like he was saying the builtins could be given an attribute to specify
-fno-finite-math-only which would only affect their specific usage?  Maybe he
just meant I could apply different flags in different translation units of my
own code, in which case I've misunderstood.  Basically, what's the "new
optimization attribute and pragma"?

Thanks!

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[matz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #32 from Michael Matz <matz at gcc dot gnu.org> 2011-10-18 01:33:10 UTC ---
To be honest, this bug report is not under any discussion anymore.  I tried to
get any sort of sanity, but in the end it's all about egos; you won't
get what you want, it's really useless to reopen this particular report.
The libstdc++ maintainers aren't as sensible as the Apple maintainers, the
middle-end maintainers aren't as useful as they should be (hiding behind less
than useful "but that's how it's documented" arguments).  And just generally
the
responses to understandable requests are more than lacking in defense of not
implementing it.

(I think it's hopeless to include even more justifications for your use cases,
it'll just be food for the reverse-trolls)

He's not a troll.  The bug-closers _are_, and the answers to closing are
severely lacking in any sort of justification.  And yes, I'm aware of past
discussions.  And no, I don't think any of it has much merit for the discussion
at hand.  They are either exaggerating (but, OMG, what to do with the
RTL "<>" -> "!=" transformation???) (answer, "nothing"), and "it's all quite
difficult", or "but currently we do builtin_isnan to 'x!=x', and we can't
recover from that" or they're not capturing the problem at all.  Many of the
justifications even just turn back to "but that's how it's documented since
<whenever>", and that's even more sorry than any other sort of "justification".
Just because a past mistake was documented as such doesn't mean it's right.

So, reporter, please go away, GCC won't fix your problem, even though it would
be reasonable to change.  Instead I'd advise you to some other more reasonable
compiler, LLVM, or maybe a commercial one.  GCC is all about defending past
mistakes in order not to change anything, not at all about being helpful to
the user.

>From the proponents of the current state of affairs I would appreciate at least
a hint of where this was "discussed to death" already, the three PRs from
Andrew certainly are not a case.  I'd speculate most of them were argued from
the point of "let's not change anything but rather argue that current behaviour
is the right one (because that's less work, although I won't say that)".

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #33 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-10-18 09:41:50 UTC ---
(In reply to comment #31)
> I can't add an attribute to the system isnan from my user code, or can I?  I've
> never been quite sure what Paolo was referring to, can someone clue me in?  It
> sounded like he was saying the builtins could be given an attribute to specify
> -fno-finite-math-only which would only affect their specific usage?  Maybe he
> just meant I could apply different flags in different translation units of my
> own code, in which case I've misunderstood.  Basically, what's the "new
> optimization attribute and pragma"?

Read the frabjous manual:
http://gcc.gnu.org/onlinedocs/gcc/Function-Specific-Option-Pragmas.html
http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html#index-g_t_0040code_007boptimize_007d-function-attribute-2496

You shouldn't need to put code in separate translation units, you can specify
different optimizations per function (though I haven't checked if that works
for -fno-finite-math-only)

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #34 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-10-18 10:03:53 UTC ---
(In reply to comment #32)
> To be honest, this bug report is not under any discussion anymore.  I tried to
> get any sort of sanity, but in the end it's all about egos; you won't
> get what you want, it's really useless to reopen this particular report.
> The libstdc++ maintainers aren't as sensible as the Apple maintainers, the
> middle-end maintainers aren't as useful as they should be (hiding behind less
> than useful "but that's how it's documented" arguments).  And just generally
> the
> responses to understandable requests are more than lacking in defense of not
> implementing it.
> 
> (I think it's hopeless to include even more justifications for your use cases,
> it'll just be food for the reverse-trolls)
> 
> He's not a troll.  The bug-closers _are_, and the answers to closing are
> severely lacking in any sort of justification.  And yes, I'm aware of past
> discussions.  And no, I don't think any of it has much merit for the discussion
> at hand.  They are either exaggerating (but, OMG, what to do with the
> RTL "<>" -> "!=" transformation???) (answer, "nothing"), and "it's all quite
> difficult", or "but currently we do builtin_isnan to 'x!=x', and we can't
> recover from that" or they're not capturing the problem at all.  Many of the
> justifications even just turn back to "but that's how it's documented since
> <whenever>", and that's even more sorry than any other sort of "justification".
> Just because a past mistake was documented as such doesn't mean it's right.
> 
> So, reporter, please go away, GCC won't fix your problem, even though it would
> be reasonable to change.  Instead I'd advise you to some other more reasonable
> compiler, LLVM, or maybe a commercial one.  GCC is all about defending past
> mistakes in order not to change anything, not at all about being helpful to
> the user.
> 
> From the proponents of the current state of affairs I would appreciate at least
> a hint of where this was "discussed to death" already, the three PRs from
> Andrew certainly are not a case.  I'd speculate most of them were argued from
> the point of "let's not change anything but rather argue that current behaviour
> is the right one (because that's less work, although I won't say that)".

Feel free to provide a patch that splits -ffinite-math-only into two pieces,
excempting optimizing the classification builtins from -ffast-math.  Please
make sure that with that code generation quality is not worse comparing
the new -ffast-math to the old -ffast-math -fno-finite-math-only (thus
make sure the classification macros are still expanded inline).  For that
to work you probably have to change RTL optimizers to not do comparison
code folding based on flag_finite_math_only (so you can expand isnan
to !(x==x) even with -ffinite-math-only).  Tree optimizers should have
already converted all these codes (so you still need to make sure to
not fold the classification builtins to comparisons at the tree level).

I would approve a patch along the above lines but I won't spend any time
on producing such patch.

I will also approve a patch that amends the current documentation
of -ffinite-math-only to say that it includes optimizing FP classification
macros.

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[ejtttje at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

--- Comment #35 from Ethan Tira-Thompson <ejtttje at gmail dot com> 2011-10-18 21:09:07 UTC ---
Thanks all for the info!

I should have realized there was literally an attribute/pragma called
'optimize' (duh), and it's already in the docs... for some reason I had gotten
the impression this was a brand new development (i.e. hadn't been released
yet), I should've checked.

FYI, the optimize attribute seems to work for fast-math, but interestingly, the
pragma doesn't.  I've created a new bug 50782 for this.

> change RTL optimizers to not do comparison code folding based on
> flag_finite_math_only (so you can expand isnan to !(x==x) even with
> -ffinite-math-only)

What would you say to a solution which allows finite math to optimize the
comparisons, but at the cost of explicit classification being full function
calls?  And of course when finite math is not in effect, everything is inlined
as normal.

This would allow core computations to be fully optimized and only explicit
classification calls would be affected.  This presumes the classification calls
are less common in order to be a good tradeoff, my intuition is that this is
the case.  It also allows those of you who want to optimize-away nan checks to
continue to do so, just use a (x!=x), and as a bonus this approach will also
work consistently between gcc variants.  What do you think?

This is also easy to implement without touching the compiler source, just apply
attributes in libstdc++ to keep the classification calls no-finite-math-only,
for example the isnan implementation would be:

#if defined(__FINITE_MATH_ONLY__) && __FINITE_MATH_ONLY__
    // apply attributes to retain classification functionality
  template<typename _Tp>
    inline typename __gnu_cxx::__enable_if<__is_arithmetic<_Tp>::__value,
                       int>::__type
    isnan(_Tp __f) __attribute__ ((optimize ("no-finite-math-only"),noinline));
#endif

  template<typename _Tp>
    inline typename __gnu_cxx::__enable_if<__is_arithmetic<_Tp>::__value,
                       int>::__type
    isnan(_Tp __f)
    {
        return std::__is_integer<_Tp>::__value ? false : __builtin_isnan(__f);
    }

(and obviously similar for fpclassify, isfinite, isinf, and isnormal)

I tweaked isnan to short circuit on __is_integer instead of a roundabout
promotion to double.  If you don't like that it can certainly go back to the
promotion style.

This works (tested with 4.6.1) because __builtin_isnan is expanded in the isnan
context, where the optimize attribute is in effect.  If you think that
expansion behavior is subject to change (see also bug 50782), we could bump
this up to apply to the builtin itself instead of the user function...?

As written, this relies on the noinline attribute trumping the inline keyword. 
We rewrite this to avoid that conflict if needed.  (Is the inline keyword
superfluous here anyway? Testing it appears so.)

[Bug middle-end/50724] cmath's floating-point classification implementations inconsistent with math.h

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |egallager at gcc dot gnu.org

--- Comment #36 from Eric Gallager <egallager at gcc dot gnu.org> ---
This bug is part of a large discussion on the llvm mailing lists, about whether
they should do similarly to GCC:
https://lists.llvm.org/pipermail/llvm-dev/2021-September/152530.html