[PATCH] math: Order signed zeros in fmin and fmax.

[Adhemerval Zanella Netto]

On 23/12/25 14:06, Wilco Dijkstra wrote:
> Hi Adhemerval/James,
> 
>>> I believe the patch results in the desired behavior on all targets,
>>> even though it modifies only the i386, i686, x86_64, and the generic C
>>> implementation.
> 
> That's good to hear, it would be useful to mention something like that in the
> description.
> 
>> As a side note, I don't think there is much gain is having the i386 version.
>> If you a felling adventurous, maybe you can send a patch to just remove the
>> i386 variants and making the i686 the default one.
> 
> I think the same is true for the x86_64 version, with my proposed improvement
> below, it generates:
> 
> "__fmin":
>         ucomisd xmm0, xmm1
>         movapd  xmm2, xmm0
>         jp      .L14
>         je      .L29
>         minsd   xmm2, xmm1
>         movapd  xmm0, xmm2
>         ret
> 
> This is from writing fmin/fmax like this:
> 
>   if (!isunordered (x, y))
>   {
>     if (__glibc_unlikely (x == y))
>       return signbit (x) ? x : y;
>     return x < y ? x : y;
>   }
> 
> This has fewer operations and branches, and allows targets to use conditional moves
> or min/max in both return cases.
> 
> The only remaining issue with the generic C implementation is that we don't yet
> inline issignaling, so the NaN handling is slightly slower. If that is inlined then the
> i686 assembler implementation could likely be removed too.

We do have an inline implementation for __issignalingf on include/math.h
and math/math.h already have a proper TG_MATH call to issignaling.  So
I think should a matter to add a double variant and maybe a per-type
one on some sysdeps/ieee754/type/header.  

> 
>> The additional targets in glibc which don't use the generic C
>> implementation are:
>> - riscv
>> - loongarch (via setting USE_F{MIN,MAX}{,F}_BUILTIN)
>> - aarch64 (via setting USE_F{MIN,MAX}{,F}_BUILTIN)
>>
>> On these architectures, the operation is already implemented by a
>> single hardware instruction with the desired behavior; no changes are
>> required.
> 
> There are more, eg. power will emit "xsmindp" since power7 at least, but they don't
> seem to define USE_F{MIN,MAX}{,F}_BUILTIN.
> 
>>> There are at least
>>> 3 different variations of fmin/fmax in the IEEE standard, so I'm wondering whether
>>> they do agree on how to handle signed zeroes... If it works on all modern targets,
>>> that's great, but that's not clear from your description because ISAs typically
>>> follow IEEE specs.
>>
>> Yes, the other variations of floating-point min/max are fminimum,
>> fminimum_mag, fminimum_num, fminimum_mag_num (and their corresponding
>> maximums). Those functions are required to order signed zeros by the C
>> standard, unlike fmin/fmax where it is only recommended.
> 
> fminmag/fmaxmag currently don't order signed zeroes either:
> 
> FLOAT
> M_DECL_FUNC (__fminmag) (FLOAT x, FLOAT y)
> {
>   FLOAT ax = M_FABS (x);
>   FLOAT ay = M_FABS (y);
>   if (isless (ax, ay))
>     return x;
>   else if (isgreater (ax, ay))
>     return y;
>   else if (ax == ay)
>     return x < y ? x : y;
>   else if (issignaling (x) || issignaling (y))
>     return x + y;
>   else
>     return isnan (y) ? x : y;
> }
> 
> This one does do it, but inefficiently (there is also different NaN handling...):
> 
> FLOAT
> M_DECL_FUNC (__fminimum_mag_num) (FLOAT x, FLOAT y)
> {
>   FLOAT ax = M_FABS (x);
>   FLOAT ay = M_FABS (y);
>   if (isless (ax, ay))
>     return x;
>   else if (isgreater (ax, ay))
>     return y;
>   else if (ax == ay)
>     return (M_COPYSIGN (1, x) <= M_COPYSIGN (1, y) ? x : y);
>   else
>     return isnan (y) ? (isnan (x) ? x + y : x) : y;
> }
> 
> So it would be great to make them consistent for signed zero and use the same efficient
> sequence in all cases.
> 
> Cheers,
> Wilco