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

On Thu, Sep 16, 2021 at 10:02 PM Cranmer, Joshua <joshua.cranmer at
intel.com>
wrote:
> 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.
>
Defining things like `isnan` is the job of libc, which often is not a part
of the compiler. It does not cause undefined behavior by itself. Redefining
macro defined in system headers may be harmful if the new macro is
inconsistent with other libc implementation (`errno` comes to mind). So
this looks like a kind of legal disclaimer. As with `-ffinite-math-only`
the redefinition is used at your own risk. For our task to remove the call
of pure function the risk is negligible. And it is needed only to reproduce
the old behavior.

>
>
> 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.
>
Regarding your example, this thread already contains consideration of this
case:

On Tue, Sep 14, 2021 at 12:50 AM Serge Pavlov <sepavloff at gmail.com>
wrote:
> On Mon, Sep 13, 2021 at 11:46 PM Chris Tetreault <ctetreau at
quicinc.com>
> wrote:
>
>> … is guaranteed to work, and I read that fast-math enables the compiler
>> to reason about constructs like `x + 0` being equal to `x`, then I’m
going
>> to be very confused when:
>>
> You are right, this was a bad idea. Compiler may optimize out `isnan` but
> only when it deduces that the value cannot be NaN, but not due to the
> user's promise. It is especially important for `isinf`. Addition of two
> finite values may produce infinity and there is no universal way to predict
> it. It is probably not an issue for types like float or double, but ML
> cores use halfs or even minifloats, where overflow is much more probable.
> If in the code:
> ```
> float r = a + b;
> if (isinf(r)) {...
> ```
> `isinf` were optimized out just because -ffinite-math-only is in effect,
> the user cannot check if overflow did not occur.
>
Rules proposed by Richard are also formulated using arguments, not results.
Now there is no intention to optimize such a case.

>
> 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.
>
The code intended for handling NaNs won't be executed in -ffinite-math-only
mode, if the mode is used correctly. So expenses are only the check itself
and the associated jump. For the code that does intensive calculation they
must be negligible. Anyway, the user can redefine `isnan`, it is as safe as
`-ffinite-math-only` itself.

>
> *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/20210917/fe236812/attachment.html>

On Thu, Sep 16, 2021 at 1:31 PM Serge Pavlov via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
>
> On Thu, Sep 16, 2021 at 10:02 PM Cranmer, Joshua <joshua.cranmer at
intel.com> wrote:
>>
>> 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.
>
>
> Defining things like `isnan` is the job of libc, which often is not a part
of the compiler.
The standard makes no real distinction between what's the job of the
compiler and what's the job of the library; it's all "the
implementation" as far as the standard is concerned. FWIW, there are
plenty of libc things which are produced by the compiler (see
https://github.com/llvm/llvm-project/tree/main/clang/lib/Headers for
all the standard library interfaces provided by Clang).
> It does not cause undefined behavior by itself.
A user-defined macro named `isnan` is UB per 7.1.3p1 if the user
includes <math.h> in the TU.
> Redefining macro defined in system headers may be harmful if the new macro
is inconsistent with other libc implementation (`errno` comes to mind). So this
looks like a kind of legal disclaimer.
I would hope so; the standard says explicitly that redefining that
macro is UB. :-)

~Aaron
> As with `-ffinite-math-only` the redefinition is used at your own risk. For
our task to remove the call of pure function the risk is negligible. And it is
needed only to reproduce the old behavior.
>
>>
>>
>>
>> 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.
>
>
> Regarding your example, this thread already contains consideration of this
case:
>
> On Tue, Sep 14, 2021 at 12:50 AM Serge Pavlov <sepavloff at
gmail.com> wrote:
>>
>> On Mon, Sep 13, 2021 at 11:46 PM Chris Tetreault <ctetreau at
quicinc.com> wrote:
>>>
>>> … is guaranteed to work, and I read that fast-math enables the
compiler to reason about constructs like `x + 0` being equal to `x`, then I’m
going to be very confused when:
>>
>> You are right, this was a bad idea. Compiler may optimize out `isnan`
but only when it deduces that the value cannot be NaN, but not due to the
user's promise. It is especially important for `isinf`. Addition of two
finite values may produce infinity and there is no universal way to predict it.
It is probably not an issue for types like float or double, but ML cores use
halfs or even minifloats, where overflow is much more probable. If in the code:
>> ```
>> float r = a + b;
>> if (isinf(r)) {...
>> ```
>> `isinf` were optimized out just because -ffinite-math-only is in
effect, the user cannot check if overflow did not occur.
>
>
> Rules proposed by Richard are also formulated using arguments, not results.
Now there is no intention to optimize such a case.
>
>>
>>
>> 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.
>
>
> The code intended for handling NaNs won't be executed in
-ffinite-math-only mode, if the mode is used correctly. So expenses are only the
check itself and the associated jump. For the code that does intensive
calculation they must be negligible. Anyway, the user can redefine `isnan`, it
is as safe as `-ffinite-math-only` itself.
>
>>
>>
>> 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
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

On Thu, Sep 16, 2021 at 1:31 PM Serge Pavlov <sepavloff at gmail.com>
wrote:
> On Tue, Sep 14, 2021 at 12:50 AM Serge Pavlov <sepavloff at
gmail.com> wrote:
>
>> On Mon, Sep 13, 2021 at 11:46 PM Chris Tetreault <ctetreau at
quicinc.com>
>> wrote:
>>
>>> … is guaranteed to work, and I read that fast-math enables the
compiler
>>> to reason about constructs like `x + 0` being equal to `x`, then
I’m going
>>> to be very confused when:
>>>
>> You are right, this was a bad idea. Compiler may optimize out `isnan`
but
>> only when it deduces that the value cannot be NaN, but not due to the
>> user's promise. It is especially important for `isinf`. Addition of
two
>> finite values may produce infinity and there is no universal way to
predict
>> it. It is probably not an issue for types like float or double, but ML
>> cores use halfs or even minifloats, where overflow is much more
probable.
>> If in the code:
>> ```
>> float r = a + b;
>> if (isinf(r)) {...
>> ```
>> `isinf` were optimized out just because -ffinite-math-only is in
effect,
>> the user cannot check if overflow did not occur.
>>
>
> Rules proposed by Richard are also formulated using arguments, not
> results. Now there is no intention to optimize such a case.
>
Infinity (HUGE_VAL) is already not NaN, so this example doesn't have
anything to do with the NaN cases being discussed.
However, let's rephrase as a NaN situation:

    bool f1(float a, float b) {
        float r = a + b;
        return isnan(r);
    }
    bool result = f1(-HUGE_VAL, HUGE_VAL);  // expect "true"

Here, `a + b` can produce quiet-NaN (if `a` is -HUGE_VAL and `b`
is +HUGE_VAL).
By Richard Smith's -ffast-math proposal as I understand it, this quiet-NaN
result would be treated "as if" it were a signaling NaN.
Under IEEE 754, no operation ever produces a signaling NaN, so
unfortunately IEEE 754 can't guide us here; but intuitively, I think
we'd
all say that merely *producing* a signaling NaN would not itself *cause* a
signal. So we store the quiet-NaN result in `r`.
Then we ask whether `isnan(r)`. The quiet-NaN result in `r` is used. By
Richard Smith's -ffast-math proposal as I understand it, any operation
*would* produce an unspecified result if it would raise a signal; but in
fact `isnan(r)` is a non-signaling operation, so even though we're treating
quiet-NaN as signaling-NaN, isnan(r) never raises any signal. So this code
has *well-defined behavior* in -ffast-math mode.
(And because the code's behavior is well-defined, therefore `isnan(r)` has
its usual meaning. When `r` holds a quiet-NaN, as in this case, `isnan(r)`
will correctly return `true`.)

I've googled, but failed to discover, whether comparison against a
signaling NaN is expected to signal. That is,

    bool f2(float a, float b) {
        float r = a + b;
        return (r != r);
    }
    bool result = f2(-HUGE_VAL, HUGE_VAL);  // expect "true" in
IEEE754
mode, but perhaps "false" in -ffast-math mode

I'm really *hoping* that comparison is a signaling operation. If it is,
then according to Richard Smith's proposal as I understand it, the compiler
would be free to optimize `(r != r)` into `(false)` in -ffast-math mode.
(And, as a corollary, the compiler would *not* generally be free to
transform `isnan(r)` into `(r != r)`, because the latter expression has
more preconditions than the former.)

–Arthur
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210916/1dd5e134/attachment.html>