Re: [PATCH 04/28] powerpc: ceil/ceilf refactor

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 04/05/2019 18:46, Gabriel F. T. Gomes wrote:
> Hi, Adhemerval,
> 
> During the review of the patch for trunc{,f}, I noticed something that
> affects this patch, as well as the patches for floor and round.
> 
> Before the changes, the POWER5+ version of the float functions perform a
> round to float (via a frsp instruction), because this is explicit in the
> assembly code, for instance:
> 
>> -ENTRY_TOCLESS (__ceilf, 4)
>> -	CALL_MCOUNT 0
>> -	frip	fp1, fp1	/* The rounding instructions are double.  */
>> -	frsp	fp1, fp1	/* But we need to set ooverflow for float.  */
>> -	blr
>> -	END (__ceilf)
> 
> When this assembly function gets replaced with the builtin, the
> generated code does not perform the round to float and will not set
> ooverflow (as mentioned in the comment above).
> 
>   $ objdump -d math/libm.so.6 | grep -E "<ceilf>:" -A 3
>   0000000000060690 <ceilf>:
>      60690:       90 0b 20 fc     frip    f1,f1
>      60694:       20 00 80 4e     blr
>           ...
> 
> I still don't know it that is actually a problem.  Do you know?  I'm
> sorry that I didn't notice this before.
> 

My understanding is the frsp was added to align the implementation to 
POSIX 2001 [1] description which contains a possible range error.
However, as described by man-pages [2], this error for IEEE 754
formats can not happen in practice (the maximum value of exponent for
float, 2^127, is larger than maximum value by significand, 2^23).

This was in fact corrected by both C99 and POSIX 2008 [3] by removing 
the possible return error. So, as gcc builtin implements, the frsp
is superfluous. 

[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/ceil.html
[2] http://man7.org/linux/man-pages/man3/ceil.3.html
[3] https://pubs.opengroup.org/onlinepubs/9699919799/functions/ceil.html