public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* Monday Patch Queue Review update (2022-01-17)
@ 2022-01-18 14:00 Carlos O'Donell
  2022-01-24 22:10 ` Joseph Myers
  0 siblings, 1 reply; 3+ messages in thread
From: Carlos O'Donell @ 2022-01-18 14:00 UTC (permalink / raw)
  To: libc-alpha

Most recent meeting status is always here:
https://sourceware.org/glibc/wiki/PatchworkReviewMeetings#Update

Meeting: 2022-01-17 @ 0900h EST (UTC-5)

Video/Audio: https://bluejeans.com/9093064454

IRC: #glibc on OFTC.

With the release upcoming we've switched to reviewing blockers and desirable changes.

 * NEW/NOBODY 290 patches
 * [v7] elf: Properly align all PT_LOAD segments [BZ #28676]
  * Adhemerval to review alignment patch.
 * linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ #28350)
  * Florian to review.
 * posix: Add terminal control setting support for posix_spawn
  * Carlos to finish review Monday.
 * Multiple rtld-audit fixes
  * Adhemerval to rebase patch set.
  * Szabolcs looked at the aarch64 pieces and gave feedback.
 * Properly handle --disable-default-pie
  * Siddhesh is going to look at this.
 * Carlos and Adhemerval discussed 64-bit time_t testing.
  * No way to change library default time_t size, just testing can test it.
 * Prelink discussion
  * Adhemerval: May I submit patches to remove prelink?
  * Carlos: Yes.
  * Discussed Intel, Arm, Linaro involvement in prelink.
  * Carlos: IHVs and ISVs need to invest upstream if they want to see prelink maintained.
 * Paul sends his regrets and includes two items:
  * I have done a review of the accuracy of the libmvec functions (in master)
    both with sse4.2, avx2 and avx512. Apart from a minor issue with atan2
    in binary64 which was fixed (#28765), I found no error larger than 4 ulps,
    both in binary32 and binary64. 
  * 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.

-- 
Cheers,
Carlos.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Monday Patch Queue Review update (2022-01-17)
  2022-01-18 14:00 Monday Patch Queue Review update (2022-01-17) Carlos O'Donell
@ 2022-01-24 22:10 ` Joseph Myers
  2022-01-25  9:27   ` Paul Zimmermann
  0 siblings, 1 reply; 3+ messages in thread
From: Joseph Myers @ 2022-01-24 22:10 UTC (permalink / raw)
  To: Paul.Zimmermann; +Cc: libc-alpha

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.

* 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.)

* 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).

* 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.

* 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.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Monday Patch Queue Review update (2022-01-17)
  2022-01-24 22:10 ` Joseph Myers
@ 2022-01-25  9:27   ` Paul Zimmermann
  0 siblings, 0 replies; 3+ messages in thread
From: Paul Zimmermann @ 2022-01-25  9:27 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha, sibid, stephane.glondu

       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/

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-01-25  9:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-18 14:00 Monday Patch Queue Review update (2022-01-17) Carlos O'Donell
2022-01-24 22:10 ` Joseph Myers
2022-01-25  9:27   ` Paul Zimmermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).