[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 27 Dec 2021 17:27:38 +0100
From: Markus Wichmann <nullplan@....net>
To: Rich Felker <dalias@...c.org>
Cc: musl@...ts.openwall.com
Subject: Re: ASM-to-C conversion for i386
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
"restrict"?
> 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.
Ciao,
Markus
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
