gcc 4.6+ compiler bug for isnan

Create issue
Issue #1121 closed
Steven R. Brandt created an issue

It seems that gcc 4.6 and up has a problem with compiling the isnan function.

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

I find that simply referring to the namespace explicitly solves it.

I had to make similar changes in Carpet/CarpetLib/src/defs.hh, but there I had to fix isinf as well. Probably the isnormal, copysign, etc. functions should get std:: for consistency.

Keyword:

Comments (14)

  1. Roland Haas
    • removed comment

    see also #1122 which is a virtual duplicate and contains the error message obseved. Affected thorns are (at least: Vectors CarpetLib).

  2. Erik Schnetter
    • removed comment

    Unfortunately, there are other C++ compilers that do not handle "std::isnan" correctly. Currently, it seems that the idiom

      using namespace std;
      isnan(x);
    

    is handled correctly by all compilers I tested. If this is not so, then we will need to introduce an autoconf test for this.

    Note also that modern GCC versions always return false from isnan() with -ffast-math (or -Ofast) in C++, but not in C.

    Note also that some of isnan's colleagues are not part of older C++ standards.

    There are also issues depending on whether <cmath> or <math.h> are included, possibly also depending on their order.

  3. Roland Haas
    • removed comment

    the using namespace std construct fails for me with ambiguous definition errors (see $1122). Explicitly using std::isnan helped but is difficult since isnan is not required in the older C++ standards it seems. I get errors from both Vectors (which is not so sophisticated) and CarpetLib's defs.hh (which seems to go to great lengths to find good versions). The error does not happen with gcc 4.7. So if it was not for a "popular" home OS being affected, I would not consider this very major.

  4. Erik Schnetter
    • removed comment

    I attach a patch that solves this problem by using autoconf to decided how to call isnan etc.

    In addition to providing a macro CCTK_ISNAN, there is also a C function CCTK_isnan defined that can be called from C++, providing a fallback if necessary.

    If C++ does not provide a proper implementation of isnan, i.e. one that can be called as std::isnan, then define a macro isnan to correct this. That is, one can now always call std::isnan to get a good definition of isnan.

    Add a test thorn TestMath.

    Remove the respective magic from Carpet.

    Update various locations that use isnan.

    Update thorn NaNChecker: - use std::isnan - simplify check_for logic - don't call C++ isnan, which is known to be optimised away in certain cases; instead, call the C isnan (via CCTK_isnan) that is not optimised away

  5. Erik Schnetter
    • changed status to open
    • removed comment

    Tested on: bethe carver datura fermi gordon gpc hopper kraken lonestar nvidia orca pandora philip queenbee ranger surveyor titan trestles zwicky.

  6. Roland Haas
    • removed comment

    I read through the patch (autoconf and all) and it seems ok. I cannot obtain the TestMath thorn or the McLachlan '_NV' thorns though it seems. I get:

    svn co https://svn.cactuscode.org/arrangements/CactusTest/TestMath TestMath
    svn: Could not open the requested SVN filesystem
    

    Also trying to compile after applying the patch, it seems as if a file cctk_Math.h seems missing from the patch.

  7. Erik Schnetter
    • removed comment

    I attached the missing files.

    The *_NV thorns are not supposed to be mentioned in the thorn list; these are non-vectorised versions of the BSSN thorns that I use for testing.

  8. Roland Haas
    • removed comment

    With the missing files I could compile on my Ubuntu (where I had problems before) test system and ran the full testsuite. Most tests pass.

    Please apply.

  9. Ian Hinder
    • removed comment

    Does the above discussion apply to isnormal, isinf and signbit as well? We have workarounds in datura.cfg for all four.

  10. Erik Schnetter
    • removed comment

    Yes, this applys it isfinite isinf isnormal signbit as well.

    The workarounds are not for C++ code, they are for the linker, since Intel and GCC disagree. The workarounds need to stay in place.

  11. Log in to comment