From: Joseph Myers <joseph@codesourcery.com>
To: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
Cc: nd <nd@arm.com>, "libc-alpha@sourceware.org" <libc-alpha@sourceware.org>
Subject: Re: Use floor functions not __floor functions in glibc libm
Date: Mon, 17 Sep 2018 21:14:00 -0000 [thread overview]
Message-ID: <alpine.DEB.2.21.1809172100400.13111@digraph.polyomino.org.uk> (raw)
In-Reply-To: <HE1PR08MB1035234F4C64C1263009541B83190@HE1PR08MB1035.eurprd08.prod.outlook.com>
On Fri, 14 Sep 2018, Wilco Dijkstra wrote:
> Looks great, thanks for doing this! The more general mechanism means
> it should be much easier to do this for the remaining functions. Yes it
> sounds like a good idea to do this for copysign too.
One thing to consider is whether we actually need this mechanism for these
functions after all, or whether we can use libm_hidden_proto /
libm_hidden_def instead.
Using this mechanism actually results in smaller and simpler patches (even
if it looks like a lot of floor etc. implementations are getting changed),
because only C implementations of the functions need to get
NO_MATH_REDIRECT added, whereas .S implementations would also need to get
libm_hidden_def added (and handling libm_hidden_* may also be more
complicated in IFUNC cases than simply disabling the redirection with
NO_MATH_REDIRECT is). For sqrt, the special mechanism is needed because,
for static linking (when libm_hidden_proto / libm_hidden_def does
nothing), we still want non-inlined calls to go to __ieee754_sqrt, not
through the wrapper that sets errno. But for the other functions, there
are no wrappers and no reason to add such wrappers (although in principle
if we do something like this for lrint or fma, while those don't have
wrappers at present there are also open bugs for missing errno setting for
those functions).
If you use libm_hidden_*, you need to consider namespace issues for static
linking. Those certainly don't apply for floor or ceil because those are
C90 functions. For rint / round / trunc you'd need to be more careful
about any uses from C90 functions (and the possibility that someone might
want to add such a use in future and run into namespace difficulties, even
if there aren't such uses at present). (In theory that applies for
copysign as well and there are even __copysign uses from C90 functions at
present in glibc - but copysign functions are in fact always inlined
except for some IBM long double cases, and long double libm functions were
only added in C99, so there wouldn't actually be namespace issues from
using libm_hidden_* for copysign functions (although also being in libc
complicates things there).)
--
Joseph S. Myers
joseph@codesourcery.com
next prev parent reply other threads:[~2018-09-17 21:14 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-14 12:57 Wilco Dijkstra
2018-09-14 13:17 ` Joseph Myers
2018-09-17 21:14 ` Joseph Myers [this message]
-- strict thread matches above, loose matches on Subject: below --
2018-09-12 12:17 Joseph Myers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.1809172100400.13111@digraph.polyomino.org.uk \
--to=joseph@codesourcery.com \
--cc=Wilco.Dijkstra@arm.com \
--cc=libc-alpha@sourceware.org \
--cc=nd@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).