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

[Goran Flegar via Phabricator via llvm-commits]

gflegar added a comment.

In D137655#3916933 <https://reviews.llvm.org/D137655#3916933>, @nikic wrote:

> In D137655#3916906 <https://reviews.llvm.org/D137655#3916906>, @gflegar wrote:
>
>> In D137655#3915463 <https://reviews.llvm.org/D137655#3915463>, @nikic wrote:
>>
>>> Doesn't this handle signed zero incorrectly?
>>
>> I believe it is:
>>
>> For `FMINIMUM`:
>>
>>   Tmp1 = -0.0, Tmp2 = +0.0  =>  Tmp1 ULT Tmp2 == True   =>  Tmp3 = Tmp1 = -0.0;     Tmp2 UO Tmp2 = False  =>  Result = Tmp3 = -0.0
>
> Isn't `-0.0 ULT 0.0` false, because negative zero and positive zero are equal?

Right, I checked the standard now. For comparisons they're considered equal, but for maximum / minimum there's a special exception that they're considered as -0.0 < 0.0. Why would anyone define this so inconsistently ... -.-
The problem for us here is performance. We would need more instructions to implement it correctly via comparisons (2 to compare + select, at least 2 for 0 handling, at least 2 for NaNs). I'm mostly concerned with the PTX backend, and for it the correct and efficient way to expand this would be to use minnum (builtin instruction), and then a NaN check. If that is true, we generate our own NaN constant (if I'm reading the standard correctly it just requires us to return a quiet NaN, which doesn't have to be the same NaN as any of the operands).
I'll try doing that instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137655