[PATCH] D52450: [Intrinsic] Add llvm.minnan and llvm.maxnan instrinsic functions

[Dan Gohman via Phabricator via llvm-commits]

sunfish added inline comments.


================
Comment at: docs/LangRef.rst:11596
+""""""""""
+If either operand is a NaN, returns NaN. If the operands compare
+equal, returns a value that compares equal to both operands. This
----------------
dschuff wrote:
> Does IEEE-754 2018 say anything about which NaN (i.e. is the payload required to be propagated too)?
No, it falls under the general rules for NaN propagation in binary operators (6.2.3). That is, it non-normatively suggests that if two NaN operands are given, one of the payloads be propagated, but it does't give a suggestion as to which.


================
Comment at: docs/LangRef.rst:11599
+means that llvm.minnan(+/-0.0, +/-0.0) could return either -0.0 or
+0.0.
+
----------------
Another thing that changed in IEEE 754-2018 is that unlike 754-2008's `minNum` and `maxNum`, 754-2018's `minimum`. `maximum`, `minimumNumber`, and `maximumNumber` are defined to treat `-0` as if it were less than `+0`. It'd be nice to adopt this new rule for these new intrinsics, however llvm.minnum already exists and doesn't follow this rule, and existing libm `fmin` and `fmax` implementations don't all follow this rule. So, it may not be practical to do so at this time. It'd be useful to mention this discrepancy with the 754-2018 rules here at least though.


Repository:
  rL LLVM

https://reviews.llvm.org/D52450