Re: [PATCH] sparc: Fix llrint and llround missing exceptions on SPARC32

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

On 15/01/24 11:38, Daniel Cederman wrote:
> On 2024-01-12 19:05, Adhemerval Zanella Netto wrote:
>>
>>
>> On 12/01/24 06:26, Daniel Cederman wrote:
>>> +/* Define these macros to 1 to workaround conversions of out-of-range
>>> +   floating-point numbers to integer types failing to raise the
>>> +   "invalid" exception, or raising spurious "inexact" or other
>>> +   exceptions.  */
>>
>> This is not really true for all sparc32, at least with sparcv9 I have
>> access I have not seen any issues with missing exception overflows or
>> spurious inexact.  Are these some leon3 limitation or issue?
>>
> 
> I don't think it is a leon specific issue. I can trigger the same issue with QEMU for other sparcs. Sparcv9 has custom versions of the functions involved so that is probably why it is not affected:
> 
> ./sysdeps/sparc/sparc32/sparcv9/fpu/multiarch/s_llrint.c
> ./sysdeps/sparc/sparc32/sparcv9/fpu/multiarch/s_llrintf.c
> 
> Corresponding patches for other 32-bit architectures references this bugzilla entry:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59412
> 
> I will have to look into this again, but I remember that it was a similar issue for sparc32.

You are right that sparcv9 has assembly specific implementation, but even
though by removing them and forcing libm to use the generic implementation
I am not seeing any regression.

I almost sure it is because on sparcv9, compile generates fdtox for double
to interger which is not subject to overflow issues.

> 
>> I think we need to know exactly how/when this happens and only enable
>> such workaound when required.
>>
>>> +#define FIX_FLT_LONG_CONVERT_OVERFLOW 0
>>> +#define FIX_FLT_LLONG_CONVERT_OVERFLOW 1
>>> +#define FIX_DBL_LONG_CONVERT_OVERFLOW 0
>>> +#define FIX_DBL_LLONG_CONVERT_OVERFLOW 1
>>> +#define FIX_LDBL_LONG_CONVERT_OVERFLOW 0
>>> +#define FIX_LDBL_LLONG_CONVERT_OVERFLOW 0
>>> +#define FIX_FLT128_LONG_CONVERT_OVERFLOW 0
>>> +#define FIX_FLT128_LLONG_CONVERT_OVERFLOW 0
>>> +
>>> +#endif /* fix-fp-int-convert-overflow.h */

So I think it should be something like:

#define FIX_FLT_LONG_CONVERT_OVERFLOW 0
#define FIX_DBL_LONG_CONVERT_OVERFLOW 0
#define FIX_LDBL_LONG_CONVERT_OVERFLOW 0
#define FIX_LDBL_LLONG_CONVERT_OVERFLOW 0
#define FIX_FLT128_LONG_CONVERT_OVERFLOW 0
#define FIX_FLT128_LLONG_CONVERT_OVERFLOW 0
/* For sparcv9 compiler generates fdtox/fstox for floating point to
   integer generation.  */
#ifdef __sparcv9
# define FIX_FLT_LLONG_CONVERT_OVERFLOW 0
# define FIX_DBL_LLONG_CONVERT_OVERFLOW 0
#else
# define FIX_FLT_LLONG_CONVERT_OVERFLOW 1
# define FIX_DBL_LLONG_CONVERT_OVERFLOW 1
#endif

Even though it is not used currently on code generation.