[PATCH] D137655: Expand fminimum and fmaximum into a pair of selects

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D137655#3917468 <https://reviews.llvm.org/D137655#3917468>, @gflegar wrote:

> In D137655#3917359 <https://reviews.llvm.org/D137655#3917359>, @nikic wrote:
>
>> I don't think this is right either. LLVM defines minnum according to the old semantics, which don't specify an order between zeroes. We'd need a separate ISD opcode for minnum according to 2018 semantics.
>
> Unfortunately, I don't have the bandwidth to chase this rabbit hole further, especially since our use case is insensitive to what happens for -0 and +0. I can add a TODO comment to fix this. Though I would still argue for submitting this, as it is correct modulo -0/+0, which is far preferable to the current state where we have a silent failure (and produce invalid code) for the backends that attempt to expand the op (like in NVPTX).

That sounds like an unrelated bug. If an operation is Expand, but we don't support expanding it, shouldn't that result in an isel failure?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137655