Re: [PATCH] fmax(), fmaxf(), fmaxl(), fmin(), fminf(), fminl() simplified

["Stefan Kanthak" <stefan.kanthak@nexgo.de>]

"Szabolcs Nagy" <nsz@port70.net> wrote:

>* Stefan Kanthak <stefan.kanthak@nexgo.de> [2019-12-11 10:55:29 +0100]:
>> Still more optimisations/simplifications in the math subtree.
>> 
>> JFTR: I'm NOT subscribed to your mailing list, so CC: me in replies!
>> 
>> --- -/src/math/fmax.c
>> +++ +/src/math/fmax.c
>> @@ -3,11 +3,9 @@
>>  double fmax(double x, double y)
>>  {
>> -        if (isnan(x))
>> +        if (x != x)
> 
> these two are not equivalent for snan input, but we dont care
> about snan, nor the compiler by default, so the compiler can
> optimize one to the other (although musl uses explicit int
> arithmetics instead of __builtin_isnan so it's a bit harder).

The latter behaviour was my reason to use (x != x) here: I attempt
to replace as many function calls as possible with "normal" code,
and also try to avoid transfers to/from FPU/SSE registers to/from
integer registers if that does not result in faster/shorter code.

> in any case the two are equivalent for practical purposes and
> using isnan better documents the intention, you should change
> the isnan definition if you think it's not efficient.
> 
>>                  return y;
>> -        if (isnan(y))
>> -                return x;
>>          /* handle signed zeros, see C99 Annex F.9.9.2 */
>> -        if (signbit(x) != signbit(y))
>> +        if (x == y)
>>                  return signbit(x) ? y : x;
>>          return x < y ? y : x;
> 
> nice trick, but the fenv behaviour is not right.

--- -/src/math/fmax.c
+++ +/src/math/fmax.c
@@ -3,11 +3,9 @@
 double fmax(double x, double y)
 {
-        if (isnan(x))
+        if (x != x)
                 return y;
-        if (isnan(y))
+        if (y != y)
                 return x;
         /* handle signed zeros, see C99 Annex F.9.9.2 */
-        if (signbit(x) != signbit(y))
+        if (x == y)
                 return signbit(x) ? y : x;
         return x < y ? y : x;
 }

> you should run any such change through libc-test
> git://repo.or.cz/libc-test and look for regressions.

I already told Rich that I neither use an OS nor a compiler/assembler
where musl or libc-test can be built.

Stefan