Chris Tetreault via llvm-dev
2021-Sep-08 18:04 UTC
[llvm-dev] [cfe-dev] Should isnan be optimized out in fast-math mode?
As a developer (who always reads the docs and generally makes good life choices), if I turn on -ffast-math, I want the compiler to produce the fastest possible floating point math code possible, floating point semantics be darned. Given this viewpoint, my opinion on this topic is that the compiler should do whatever it wants, given the constraints of the documented behavior of NaN. I think the clang docs for -ffast-math are pretty clear on this subject: ``` Enable fast-math mode. This option lets the compiler make aggressive, potentially-lossy assumptions about floating-point math. These include: ... - Operands to floating-point operations are not equal to NaN and Inf ... ``` The compiler may assume that operands to floating point operations are not NaN or infinity. So: - What should return `std::numeric_limits<double>::has_quiet_NaN()`? : It should return true if it would have returned true with fast math disabled. Clang is not required to pretend NaN doesn't exist, it's allowed to pretend arguments cannot be NaN if that is convenient. - What body should have this function if it is used in a program where some functions are compiled with `fast-math` and some without? : This function should be allowed to act as if NaN exists in all cases. - Should inlining of a function compiled with `fast-math` to a function compiled without it be prohibited in inliner? No. The author of the function that uses fast-math made their choices, and the user of that function should have vetted their dependencies better. In my view, this is no different than if somebody wrote `if (x == y/z) ...`; it's a bug on the user. It's not clang's fault that this code doesn't work as the author wanted. - Should `std::isnan(std::numeric_limits<float>::quiet_NaN())` be true? : No. quiet_NaN() can return whatever it wants, but the call to std::isnan can be replaced with false since it may assume it's argument is not NaN. Of course, this all sounds fine and well, but the reality is that people don't read docs and don't make good life choices. They turn on fast math because they want it to reduce `x * 0` to `0`, and are surprised when their NaN handling code fails. This is unfortunate, but I don't think we should reduce the effectiveness of fast-math because of this human issue. Other flags exist for these users, and when they complain they should be told about them. Really this is an issue of poor developer discipline, and if we really want to solve this, perhaps some sort of "fast math sanitizer" can be created. It can statically analyze code and complain when it sees things like `if (isnan(foo))` not guarded by `__FAST_MATH__` with mast math enabled. Or, maybe the compiler can just issue a warning unconditionally in this case. Thanks, Chris Tetreault From: cfe-dev <cfe-dev-bounces at lists.llvm.org> On Behalf Of Serge Pavlov via cfe-dev Sent: Wednesday, September 8, 2021 10:03 AM To: LLVM Developers <llvm-dev at lists.llvm.org>; Clang Dev <cfe-dev at lists.llvm.org> Subject: [cfe-dev] Should isnan be optimized out in fast-math mode? WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros. Hi all, One of the purposes of `llvm::isnan` was to help preserve the check made by `isnan` if fast-math mode is specified (https://reviews.llvm.org/D104854). I'd like to describe reason for that and propose to use the behavior implemented in that patch. The option `-ffast-math` is often used when performance is important, as it allows a compiler to generate faster code. This option itself is a collection of different optimization techniques, each having its own option. For this topic only the option `-ffinite-math-only` is of interest. With it the compiler treats floating point numbers as mathematical real numbers, so transformations like `0 * x -> 0` become valid. In clang documentation (https://clang.llvm.org/docs/UsersManual.html#cmdoption-ffast-math) this option is described as: "Allow floating-point optimizations that assume arguments and results are not NaNs or +-Inf." GCC documentation (https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html) is a bit more concrete: "Allow optimizations for floating-point arithmetic that assume that arguments and results are not NaNs or +-Infs." **What is the issue?** C standard defines a macro `isnan`, which can be mapped to an intrinsic function provided by the compiler. For both clang and gcc it is `__builtin_isnan`. How should this function behave if `-ffinite-math-only` is specified? Should it make a real check or the compiler can assume that it always returns false? GCC optimizes out `isnan`. It follows from the viewpoint that (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724#c1): "With -ffinite-math-only you are telling that there are no NaNs and thus GCC optimizes isnan (x) to 0." Such treatment of `-ffinite-math-only` has sufficient drawbacks. In particular it makes it impossible to check validity of data: a user cannot write assert(!isnan(x)); because the compiler replaces the actual function call with its expected value. There are many complaints in GCC bug tracker (for instance https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949 or https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50724) as well as in forums (https://stackoverflow.com/questions/47703436/isnan-does-not-work-correctly-with-ofast-flags or https://stackoverflow.com/questions/22931147/stdisinf-does-not-work-with-ffast-math-how-to-check-for-infinity). Proposed solutions are using integer operations to make the check, to turn off `-ffinite-math-only` in some parts of the code or to ensure that libc function is called. It clearly demonstrates that `isnan` in this case is useless, but users need its functionality and do not have a proper tool to make required checks. The similar direction was criticized in llvm as well (https://reviews.llvm.org/D18513#387418). **Why imposing restrictions on floating types is bad?** If `-ffinite-math-only` modifies properties of `double` type, several issues arise, for instance: - What should return `std::numeric_limits<double>::has_quiet_NaN()`? - What body should have this function if it is used in a program where some functions are compiled with `fast-math` and some without? - Should inlining of a function compiled with `fast-math` to a function compiled without it be prohibited in inliner? - Should `std::isnan(std::numeric_limits<float>::quiet_NaN())` be true? If the type `double` cannot have NaN value, it means that `double` and `double` under `-ffinite-math-only` are different types (https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544641.html). Such a way can solve these problems but it is so expensive that hardly it has a chance to be realized. **The solution** Instead of modifying properties of floating point types, the effect of `-ffinite-math-only` can be expressed as a restriction on operation usage. Actually clang and gcc documentation already follows this way. Fast-math flags in llvm IR also are attributes of instructions. The only question is whether `isnan` and similar functions are floating-point arithmetic. From a practical viewpoint, treating non-computational functions as arithmetic does not add any advantage. If a code extensively uses `isnan` (so could profit by their removal), it is likely it is not suitable for -ffinite-math-only. This interpretation however creates the problems described above. So it is profitable to consider `isnan` and similar functions as non-arithmetical. **Why is it safe to leave `isnan`?** The probable concern of this solution is deviation from gcc behavior. There are several reasons why this is not an issue. 1. -ffinite-math-only is an optimization option. A correct program compiled with -ffinite-math-only and without it should behave identically, if conditions for using -ffinite-math-only are fulfilled. So making the check cannot break functionality. 2. `isnan` is implemented by libc, which can map it to a compiler builtin or use its own implementation, depending on configuration options. `isnan` implemented in libc obviously always does the real check. 3. ICC and MSVC preserve `isnan` in fast-math mode. The proposal is to not consider `isnan` and other such functions as arithmetic operations and do not optimize them out just because -ffinite-math-only is specified. Of course, there are cases when `isnan` may be optimized out, for instance, `isnan(a + b)` may be optimized if -ffinite-math-only is in effect due to the assumption (result of arithmetic operation is not NaN). What are your opinions? Thanks, --Serge -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210908/8c9622f9/attachment-0001.html>
Joerg Sonnenberger via llvm-dev
2021-Sep-08 20:58 UTC
[llvm-dev] [cfe-dev] Should isnan be optimized out in fast-math mode?
On Wed, Sep 08, 2021 at 06:04:08PM +0000, Chris Tetreault via llvm-dev wrote:> As a developer (who always reads the docs and generally makes good life > choices), if I turn on -ffast-math, I want the compiler to produce the > fastest possible floating point math code possible, floating point > semantics be darned. Given this viewpoint, my opinion on this topic is > that the compiler should do whatever it wants, given the constraints of > the documented behavior of NaN.There is a huge different between optimisations that assume NaN is not present and breaking checks for them. I'm not convinced at all that constant-folding isnan to false will actually speed up real world code. Joerg
Jorg Brown via llvm-dev
2021-Sep-20 18:29 UTC
[llvm-dev] [cfe-dev] Should isnan be optimized out in fast-math mode?
On Wed, Sep 8, 2021 at 11:04 AM Chris Tetreault via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Of course, this all sounds fine and well, but the reality is that people > don't read docs and don't make good life choices. They turn on fast math > because they want it to reduce `x * 0` to `0`, and are surprised when their > NaN handling code fails. This is unfortunate, but I don't think we should > reduce the effectiveness of fast-math because of this human issue. Other > flags exist for these users, and when they complain they should be told > about them. Really this is an issue of poor developer discipline, and if we > really want to solve this, perhaps some sort of "fast math sanitizer" can > be created. It can statically analyze code and complain when it sees things > like `if (isnan(foo))` not guarded by `__FAST_MATH__` with fast math > enabled. Or, maybe the compiler can just issue a warning unconditionally in > this case. >A "fast math sanitizer" sounds like a GREAT idea. Also, I'd say that if -ffast-math is on, and -fsanitize=undefined, then the UB detection should already be doing this. One caveat, though. Suppose there is code like this: if (d == std::numeric_limits<double>::infinity()) { Does the comparison against infinity count as UB, since the compiler is allowed to assume there are no infinite fp values? I hope not, or an optimization I'm about to check in will have to be reverted. -- Jorg -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210920/e3487714/attachment.html>