Re: Monday Patch Queue Review update (2022-01-17)

[Paul Zimmermann <Paul.Zimmermann@inria.fr>]

       Dear Joseph,

thank you very much for your feedback, this is extremely useful!

> Date: Mon, 24 Jan 2022 22:10:34 +0000
> From: Joseph Myers <joseph@codesourcery.com>
> 
> On Tue, 18 Jan 2022, Carlos O'Donell via Libc-alpha wrote:
> 
> >   * we are making good progress in the CORE-MATH project: 17 out of 26
> >     functions are now available in single precision (binary32).
> >     See https://homepages.loria.fr/PZimmermann/CORE-MATH/.
> >     In most cases the correctly-rounded CORE-MATH code outperforms the
> >     GNU libc one (see the "perf" graphs). The only exceptions so far are
> >     the cosf function on i7, and the expf function.
> 
> A few comments based on spot checks of the functions there:
> 
> * Many of the functions are severely lacking in comments, I'd expect 
> detailed comments throughout the functions explaining what is going on.

we'll try to add more comments

> * Many of the functions seem specific to 64-bit systems, with e.g.
> 
> typedef union {double f; unsigned long u;} b64u64_u;
> 
> making an assumption on the size of long.  You should use standard types 
> from <stdint.h> such as uint32_t and uint64_t throughout whenever you need 
> a given integer width, which probably means avoiding "long" everywhere.  
> (Likewise, avoid local typedef names such as u64.)

indeed, uint64_t would be better than "unsigned long" in this example

> * Use of __int128 (e.g. in cosf) is also specific to 64-bit systems; GCC 
> doesn't support it on 32-bit systems.  Code needs to be written so as to 
> work on 32-bit systems as well as 64-bit (if that means different code 
> paths, you should probably replicate the testing for being correctly 
> rounded on both 32-bit and 64-bit systems, as well as for other variations 
> such as architectures where GCC fuses multiply and add into fma unless you 
> use -ffp-contract=off).

indeed, we will probably also provide code running on 32-bit systems.

> * Correct underflow, overflow and inexact exceptions are required for 
> cr_*; it would be a good idea to check for that in your exhaustive 
> binary32 testing (and fix the implementations accordingly) - though 
> covering both before-rounding and after-rounding tininess detection cases, 
> for functions where some inputs give results in the relevant narrow 
> intervals for which those are different, would require testing on multiple 
> architectures, and in practice it might be easier for us to make sure to 
> add all such inputs to the glibc testsuite and test any proposed 
> integration on such architectures.

we are aware of this

> * I'm not sure what the coding style in these functions is meant to be, 
> but given e.g. all the other functions in the files not suitable for 
> inclusion in glibc as-is my assumption is we'd want to reformat anything 
> included in glibc into GNU style.

clearly the code we propose will need to be reformated by the developers
of the different math libraries with their respective coding style.

Best regards,
Paul

PS: the CORE-MATH page moved to https://core-math.gitlabpages.inria.fr/