mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Markus Wichmann <>
To: Rich Felker <>
Subject: Re: [musl] ASM-to-C conversion for i386
Date: Mon, 27 Dec 2021 17:27:38 +0100	[thread overview]
Message-ID: <20211227162738.GD1949@voyager> (raw)
In-Reply-To: <>

On Mon, Dec 27, 2021 at 10:00:11AM -0500, Rich Felker wrote:
> On Sun, Dec 26, 2021 at 09:42:38PM +0100, Markus Wichmann wrote:
> > For the maths code,
> One thing I found right away on the math code: You can't use "=t" for
> output in a float or double variable unless the asm naturally
> guarantees there's no excess precision (e.g. fmod where the operation
> is exact and the precision is necessarily bounded by the input
> precision). Otherwise you need "=t" to a temporary long double output
> to then let the compiler convert down to float or double (e.g. at time
> of return; I think we're still using explicit cast there too in case
> some compilers get this wrong).

Oh yes, I noticed the missing conversion instructions in some places and
didn't in others. As I said, some cleanup is in order. The branch is not
currently ripe for merging (yet). The code was written over several days
in various stages of alertness and distraction.

> This was probably a compromise between the "likely" notation I don't
> like (reads as a predicate lik "if this is likely" rather than "if X,
> which is likely") and my rather odd suggestion of as_expected (if
> (as_expected(x))). :-P

I agree to the point, although likely()/unlikely() seems to be the
convention everywhere else. OK, so predict_X it is. Which is still in
libm.h, so I cannot use it anywhere else in the code.

> > Most of that code was straightforward, but some of the more complex
> > functions I am not sure about. What is up with __exp2l()? I can see that
> > expl() is calling it, but I'm not sure why. But its existence forced me
> > to employ a technique not used elsewhere in the code (that I could
> > find): A hidden alias. I vaguely recall that such hackery was rejected
> > before (on grounds of old binutils reacting badly to such magic), but I
> > don't really know what else I could have done. Or was the correct way to
> > make __exp2l() a hidden function with the actual implementation and
> > exp2l() (without the underscores) a weak alias?
> Here it doesn't really matter at all since they're both in the
> baseline C namespace.

I found later that you introduced the hidden double-underscore versions
of some symbols that are called directly from assembler elsewhere to get
rid of things that would be textrels if not for a linker option. So that
entire point is moot with the move to C, anyway. I'll remove those in
the math code.

In the string code, I have currently defined __memcpy_fwd this way. Not
sure what to do there, yet, and the way I'm using the definition in
memmove() might invoke undefined behavior, because of
declaration/definition mismatch. Or do I define __memcpy_fwd without

> So I think
> having them all share an inline function is fine. Of course since
> there's then no reason for them to be in the same source file, they
> should probably be moved to their corresponding named files instead of
> empty .c files.

So, you want me to move the core function to a header file as static
inline? That is possible and avoids the source duplication problem. I
was also wondering about those functions where the smaller-precision
versions just do what the long-double version is doing with maybe some
pre-/post-processing. Should they receive the same treatment or should
they just call the long-double version? E.g. acos, asin, log2, &c. Not
sure if the savings from not having to do a proper call/return are worth
the hassle of a new file with some of these.

> > Speaking of, how would you like those? One patch for everything, one
> > patch per directory (i.e. one for thread, one for math, one for fenv,
> > one for string), or one per functions group (the three precisions of
> > each function), or one per function? I don't want to overwhelm you.
> Probably split up somewhat so that things that would need discussion
> separately or that could be conceptually right/wrong independent of
> one another (not just minor bugs) are separate commits. After looking
> at the commits in your repo more I can probably make a better
> recommendation.

OK, I'll use my judgment when the time comes.


  reply	other threads:[~2021-12-27 17:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-26 20:42 Markus Wichmann
2021-12-26 21:20 ` Markus Wichmann
2021-12-27 13:08 ` Markus Wichmann
2021-12-27 15:00 ` Rich Felker
2021-12-27 16:27   ` Markus Wichmann [this message]
2021-12-27 16:30   ` Rich Felker
2021-12-27 18:04     ` Markus Wichmann
2021-12-27 18:41       ` Rich Felker
2021-12-28  8:42         ` Markus Wichmann
2021-12-29 10:02   ` Markus Wichmann

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211227162738.GD1949@voyager \ \ \ \

* 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.
Code repositories for project(s) associated with this public inbox

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 NNTP newsgroup(s).