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!
