[PATCH] D137655: Expand fminimum/fmaximum into fminnum/fmaxnum + NaN check
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 10 10:56:55 PST 2022
arsenm added a comment.
In D137655#3919749 <https://reviews.llvm.org/D137655#3919749>, @gflegar wrote:
> 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).
Hard disagree
> And we're not talking about the new semantics for FMINIMUM/FMAXIMUM, but for the new semantics of FMINNUM/FMAXNUM (the +/-0 handling changed).
The 2019 final spec does not have minnum or maxnum; they were removed and replaced with minimum and maximum which have specified signed zero behavior. Unless there was a draft revision I missed, there was never a defined minnum with specified -0 behavior. It would be helpful to define minnum/maxnum variants with specified -0 ordered less than +0
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137655/new/
https://reviews.llvm.org/D137655
More information about the llvm-commits
mailing list