[PATCH] D104854: Introduce intrinsic llvm.isnan

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added a comment.

In D104854#2957735 <https://reviews.llvm.org/D104854#2957735>, @kpn wrote:

> In D104854#2957490 <https://reviews.llvm.org/D104854#2957490>, @lebedev.ri wrote:
>
>> In D104854#2957471 <https://reviews.llvm.org/D104854#2957471>, @sepavloff wrote:
>>
>>> In D104854#2957423 <https://reviews.llvm.org/D104854#2957423>, @spatel wrote:
>>>
>>>> Is it intentional that we are not canonicalizing the intrinsic call back to `fcmp uno` in the default FP environment?
>>>
>>> It is lowered to unordered comparison by default. Changing `llvm.isnan` to  `fcmp uno` somewhere in IR would make it possible to optimize out the latter if fast-math mode is on. Preserving semantics of `isnan` when fast-math is in effect was one of the goals of this change.
>>
>> Eeek. Was there an RFC about this?
>> This does not sound good to me at all,
>> much like "let's not apply fast-math flags to x86 vector intrinsics".
>
> We can switch into and out of the default FP environment inside a single function.

Really? The constrained intrinsic documentation claims the reverse (https://llvm.org/docs/LangRef.html#constrainedfp):

> If any FP operation in a function is constrained then they all must be constrained. This is required for correct LLVM IR. Optimizations that move code around can create miscompiles if mixing of constrained and normal operations is done. The correct way to mix constrained and less constrained operations is to use the rounding mode and exception handling metadata to mark constrained intrinsics as having LLVM’s default behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104854