[PATCH] D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check

Thu Nov 10 09:14:51 PST 2022

gflegar added a comment.

In D137655#3919674 <https://reviews.llvm.org/D137655#3919674>, @arsenm wrote:

> In D137655#3918964 <https://reviews.llvm.org/D137655#3918964>, @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.
>>
>> Interestingly though, for the NVPTX backend this will end up producing the correct code, since minnum is lowered to the min/max PTX instructions, which defines the 2018 semantics. (I do agree though that the intermediate code is not correct.)
>
> It's not OK to have wrong intermediate code. We do have the "new" semantic opcodes already in FMINIMUM/FMAXIMUM

It's even less OK to fail altogether (which is what is happening without this patch). And we're not talking about the new semantics for FMINIMUM/FMAXIMUM, but for the new semantics of FMINNUM/FMAXNUM (the +/-0 handling changed).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137655