[PATCH] D104854: Introduce intrinsic llvm.isnan

Eli Friedman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 26 13:16:33 PDT 2021


efriedma added a comment.

In D104854#2904826 <https://reviews.llvm.org/D104854#2904826>, @sepavloff wrote:

> In D104854#2886328 <https://reviews.llvm.org/D104854#2886328>, @efriedma wrote:
>
>>> The options '-ffast-math' and '-fno-honor-nans' imply that FP operation
>>> operands are never NaNs. This assumption however should not be applied
>>> to the functions that check FP number properties, including 'isnan'. If
>>> such function returns expected result instead of actually making
>>> checks, it becomes useless in many cases.
>>
>> This doesn't work the way you want it to, at least given the way nnan/ninf are currently defined in LangRef.  It's possible to end up in a situation where `isnan(x) == isnan(x)` evaluates to false at runtime.  It doesn't matter how you compute isnan; the problem is that the input is poisoned.
>>
>> I think the right solution to this sort of issue is to insert a "freeze" in the LLVM IR, or something like that.  Not sure how we'd expect users to write this in C.  Suggestions welcome.
>
> According to the documentation, nnan/ninf may be applied to `fneg`, `fadd`, `fsub`, `fmul`, `fdiv`, `frem`, `fcmp`, `phi`, `select` and `call`. We can ignore this flag for calls of isnan and similar functions. Of course, if conditions of using `-ffast-math` are broken, we have undefined behavior and `isnan(x) != isnan(x)` becomes possible, like in this code:

Right... so how can you produce a NaN in these circumstances?  You could load one from memory, I guess?

It would probably be a good idea to have an instcombine that combines away isnan on a value produced by an operation marked nnan, so we don't confuse people reading assembly into assuming isnan is actually reliable in that context.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104854/new/

https://reviews.llvm.org/D104854



More information about the cfe-commits mailing list