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.