llvm dev - Sep 2021 - [cfe-dev] Should isnan be optimized out in fast-math mode?

Let me make some summary. I omit references for brevity, they are spread in
the thread.

Treatment of `isnan` with `-ffinite-math-only` has issues:
- There are many users' complaints and disagreement expressed in GCC bug
tracker and forums about the treatment.
- There are legitimate use cases when `isnan` needs to be called in
`-ffinite-math-only` mode.
- Users have to invent workarounds to get functionality of `isnan`, which
results in portability and performance loss.
- There is inconsistency with the behavior of libc, which always does a
real check, and the compiler, which omits it.
Preserving `isnan` in the code would solve all of them.

What is the risk?

`-ffinite-math-only` is an optimization option, so preserving `isnan`
cannot break the behavior of correct programs. The only possible negative
impact is some loss of performance. It is unlikely that a real program
spends so much time in `isnan` calls that it has noticeable effect, but if
it does, a user can conditionally redefine `isnan` macro.

Preserving `isnan` in `-ffinite-math-only` mode is safe and makes the
compiler more reliable and user-friendly.

Thanks,
--Serge
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210916/99e16ec1/attachment.html>

On Thu, Sep 16, 2021 at 1:37 AM Serge Pavlov via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
>
> Let me make some summary. I omit references for brevity, they are spread in
the thread.
>
> Treatment of `isnan` with `-ffinite-math-only` has issues:
> - There are many users' complaints and disagreement expressed in GCC
bug tracker and forums about the treatment.
> - There are legitimate use cases when `isnan` needs to be called in
`-ffinite-math-only` mode.
> - Users have to invent workarounds to get functionality of `isnan`, which
results in portability and performance loss.
> - There is inconsistency with the behavior of libc, which always does a
real check, and the compiler, which omits it.
> Preserving `isnan` in the code would solve all of them.
>
> What is the risk?
>
> `-ffinite-math-only` is an optimization option, so preserving `isnan`
cannot break the behavior of correct programs. The only possible negative impact
is some loss of performance. It is unlikely that a real program spends so much
time in `isnan` calls that it has noticeable effect, but if it does, a user can
conditionally redefine `isnan` macro.
>
> Preserving `isnan` in `-ffinite-math-only` mode is safe and makes the
compiler more reliable and user-friendly.
FWIW, I personally come down on the side of *not* removing the call to
isnan() that the user explicitly wrote. It's not beyond belief that a
C API is called from another language that can generate a NaN, and a
user who has enabled finite math only may still wish to guard against
those cross-module cases passing in a NAN that they know their TU
can't properly handle.

~Aaron
>
> Thanks,
> --Serge
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

I think you are not adequately summing up the risks of your approach; there are
three other issues I see.

First, redefining `isnan` as a macro is expressly undefined behavior in C (see
section 7.1.3, clauses 2 and 3—it’s undefined behavior to define a macro as a
same name as reserved identifier in a standard library header). Conditionally
redefining an `isnan` macro is therefore not a permissible solution.

The second thing that has been repeatedly brought up that is missing is the fact
that `isnan` may still be inconsistently optimized out. `isnan(x)` would only be
retained in the program if the compiler cannot deduce that `x` is the result of
a nnan arithmetic operation. If it can deduce that—the simplest case being the
somewhat questionable `isnan(x + 0)` example, but it’s also possible that, e.g.,
you’re calling `isnan(sum)` on the result of a summation, which would be the
result of an arithmetic expression post-mem2reg/SROA—then the compiler would
still elide it. It could be that this is less surprising to users than
unconditionally optimizing array `isnan(x)`, but it should still be admitted
that there is a potential for surprise here.

A final point is that the potential optimization benefits of eliding `isnan` are
not limited to the cost of running the function itself (which are likely to be
negligible), but also include the benefits of deleting any subsequent code that
is attempting to handle NaN values, which may be fairly large blocks. A salient
example is complex multiplication and division, where the actual expansion of 
the multiplication and division code itself is dwarfed by the recalculation code
if the result turns out to be a NaN.

From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Serge
Pavlov via llvm-dev
Sent: Thursday, September 16, 2021 1:37
To: Chris Tetreault <ctetreau at quicinc.com>
Cc: Arthur O'Dwyer <arthur.j.odwyer at gmail.com>; llvm-dev at
lists.llvm.org; cfe-dev at lists.llvm.org
Subject: Re: [llvm-dev] [cfe-dev] Should isnan be optimized out in fast-math
mode?

Let me make some summary. I omit references for brevity, they are spread in the
thread.

Treatment of `isnan` with `-ffinite-math-only` has issues:
- There are many users' complaints and disagreement expressed in GCC bug
tracker and forums about the treatment.
- There are legitimate use cases when `isnan` needs to be called in
`-ffinite-math-only` mode.
- Users have to invent workarounds to get functionality of `isnan`, which
results in portability and performance loss.
- There is inconsistency with the behavior of libc, which always does a real
check, and the compiler, which omits it.
Preserving `isnan` in the code would solve all of them.

What is the risk?

`-ffinite-math-only` is an optimization option, so preserving `isnan` cannot
break the behavior of correct programs. The only possible negative impact is
some loss of performance. It is unlikely that a real program spends so much time
in `isnan` calls that it has noticeable effect, but if it does, a user can
conditionally redefine `isnan` macro.

Preserving `isnan` in `-ffinite-math-only` mode is safe and makes the compiler
more reliable and user-friendly.

Thanks,
--Serge
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210916/98dbd2b9/attachment.html>