7
\$\begingroup\$

I live in hostile environments (GCC and Clang) which remove calls to isnan which prevents me to perform validation of my input before running computations.

Here is a potential replacement: is this a correct implementation of isnan (no matter whether the NaN is quiet or signaling - I am reading random data from untrusted sources and have to filter anything that is not a real number) ?

constexpr bool safe_isnan(double val) noexcept
{
  const auto x = std::bit_cast<uint64_t>(val);

  return (x >= 0x7FF0000000000001ull && x <= 0x7FFFFFFFFFFFFFFFull)
      || (x >= 0xFFF0000000000001ull)
  ;
}
\$\endgroup\$
1
  • \$\begingroup\$ Comments are not for extended discussion; this conversation has been moved to chat. \$\endgroup\$
    – Mast
    Dec 17, 2021 at 16:44

3 Answers 3

13
\$\begingroup\$

First of all, the compiler is not hostile; by using -ffast-math you are asking for an optimization that the compiler dutifully performs. Perhaps it is not what you desired the compiler would do in this particular case, but please don't attribute that to maliciousness from the compiler or the compiler developers.

Compilers routinely are able to look at code patterns and see that a piece of code does the same as some built-in function, and replace one with the other, and then perform more optimizations on it. So in theory, the compiler might decide that what you are doing is the same as std::isnan(). Luckily, current versions of GCC and Clang don't optimize your safe_isnan() away.

Also be aware that as part of -ffast-math optimizations, an operation that would have resulted in a NaN might now return a finite value. safe_isnan() is not going to protect you against that. So it might work for values you read directly from disk or network, but be aware of this when using it on the results of arithmetic operations.

\$\endgroup\$
7
\$\begingroup\$

You have missed the namespace qualifier of std::uint64_t. It seems that std::uint_fast64_t should be sufficient, to make the code more portable. The exact-width type is probably appropriate, but we should confirm that using static_assert(sizeof (double) == sizeof (std::uint64_t).

We probably want to ensure that the types have the same endianness - probably the easiest way to do that is follow the definition with a compile-time test like

static_assert(safe_isnan(std::numeric_limits<double>::quiet_NaN()));

It looks like the test could be simplified. Instead of those >= and <= comparisons, just check the relevant bits are set:

auto constexpr ones_mask = 0x7ff0000000000000;
auto constexpr zero_mask = 0x000fffffffffffff;

return (x & ones_mask) == ones_mask  &&  (x & zero_mask) != 0;

We should probably conditionalise on std::numeric_limits<double>::is_iec559 since we depend on the floating-point layout.

The whole thing should be unnecessary - but if you've you've selected -ffast-math, then your compiler can assume that these bit patterns are impossible. You probably need to compile this function separately without that option for it to work reliably. But in that case, you might as well just compile a tiny (non-fastmath) wrapper that ensures the standard isnan() is called:

bool safe_isnan(double val) noexcept { return std::isnan(val); }

(Note: not constexpr, and take steps to avoid inlining - or link-time optimisation)

Another alternative is to use (or reimplement) std::fpclassify() and use that to implement your versions of isnan(), isfinite(), etc.

\$\endgroup\$
14
  • 3
    \$\begingroup\$ Is std::uint_fast64_t really a better choice, though? It could be wider than double, which would break std::bit_cast. Since these constants are designed for IEEE754 binary64, it's fine (and maybe better) to require an integer type that's exactly 64-bit. \$\endgroup\$ Dec 18, 2021 at 2:37
  • 1
    \$\begingroup\$ Something like (x | 0x8000000000000000ull) > 0xfff0000000000000ull might be even more efficient. \$\endgroup\$ Dec 18, 2021 at 5:48
  • 2
    \$\begingroup\$ @IlmariKaronen: or x<<1 to discard the sign bit instead of forcing it to 1. Good idea, on many ISAs that would avoid needing another 64-bit constant. (x86-64 has bts that can set a single bit, but on x86-64 ANDing with ones_mask would require a 10-byte mov r64, imm64 / 3-byte and r64, r64. AArch64 bit-range immediates let it AND or OR in one instruction, but just as easily shift.) Right-shifting to the bottom would allow an easy AND or OR immediate and an immediate compare. \$\endgroup\$ Dec 18, 2021 at 9:18
  • 1
    \$\begingroup\$ @IlmariKaronen's suggestion does compile the best for ARM64 and x86-64, especially with x86-64 gcc. godbolt.org/z/bcz84a1v3 Toby's version compiles with a branch instead of cmov for the original from the question. Also included a version of my right-shift idea, returning an integer 0 / non-zero that the caller can test/branch on without materializing a boolean 0 / 1. We can just return x << 12 (i.e. the mantissa bits) if the exponent is all-ones, otherwise return 0. That avoids large constants, but the code-size ends up worse than Ilmari's I think. (Especially Ilmari's w. x<<1 \$\endgroup\$ Dec 18, 2021 at 9:43
  • 2
    \$\begingroup\$ One may want, if there's no way to prevent inlining/LTO/etc., to write the value into a volatile and then read it back. Then the compiler won't be able to make assumptions on the value read back, and won't optimize away the custom check for NaN. \$\endgroup\$
    – Ruslan
    Dec 18, 2021 at 22:48
0
\$\begingroup\$

Minor: ll serves no purpose here

DRYer code
ll is useful to force a minimum width type. Unnecessary with all same sign type compares.

const auto x = std::bit_cast<uint64_t>(val);

// return (x >= 0x7FF0000000000001ull && x <= 0x7FFFFFFFFFFFFFFFull)
//     || (x >= 0xFFF0000000000001ull);
return (x >= 0x7FF0000000000001u && x <= 0x7FFFFFFFFFFFFFFFu)
    || (x >= 0xFFF0000000000001u);

... or re-formed as

return (x & 7FFFFFFFFFFFFFFFu) >= 0x7FF0000000000001u;
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.