Date: Tue, 10 Dec 2019 17:13:00 -0800 From: Rosen Penev <rosenp@...il.com> To: musl@...ts.openwall.com Cc: Stefan Kanthak <stefan.kanthak@...go.de> Subject: Re: More patches for math subtree On Tue, Dec 10, 2019 at 2:17 PM Rich Felker <dalias@...c.org> wrote: > > On Tue, Dec 10, 2019 at 10:32:26PM +0100, Stefan Kanthak wrote: > > "Rich Felker" <dalias@...c.org> wrote: > > > > > > > On Tue, Dec 10, 2019 at 05:57:55PM +0100, Stefan Kanthak wrote: > > >> Some more optimisations: the current implementations of ceil(), floor() > > >> and trunc() for i386 change the rounding control using fldcw instructions, > > >> which are SLOW; these patches provide faster and smaller branch-free (!) > > >> implementations. > > >> > > >> JFTR: I'm NOT subscribed to your mailing list, so CC: me in replies! > > >> > > >> --- -/src/math/i386/floor.s > > >> +++ +/src/math/i386/floor.s > > >> @@ -1,67 +1,26 @@ > > >> .global floorf > > >> .type floorf,@function > > >> floorf: > > >> flds 4(%esp) > > >> jmp 1f > > >> > > >> .global floorl > > >> .type floorl,@function > > >> floorl: > > >> fldt 4(%esp) > > >> jmp 1f > > >> > > >> .global floor > > >> .type floor,@function > > >> floor: > > >> fldl 4(%esp) > > >> +1: fld %st(0) > > >> + frndint > > >> + fxch %st(1) > > >> + fucomip %st(1),%st(0) > > >> + fld1 > > >> + fldz > > >> + fcmovb %st(1),%st(0) > > > ^^^^^^ > > > > > > fcmovb is not in the baseline ISA. > > > > This is but irrelevant or inconsequent: FCMOV* as well as FCOMI* and > > FUCOMI* were introduced with the PentiumPro. If you allow the use of > > the latter you can safely use the former too. And FCOMI* and FUCOMI* > > are already used in other .S files. > > This is why we're not using them. I think you're looking at x86_64 > where they are in the baseline ISA. > > > > Otherwise, I *think* the idea of this patch looks good, provided I'm > > > not missing anything with respect to how status flags are affected. > > > > FRNDINT takes care of them! > > OK. > > > > As noted in the other email (sorry about not CC'ing you before; I've > > > got you on CC now), I really want to get rid of all these .s files in > > > favor of __asm__ statements with proper constraints in C source files. > > > That makes them inlineable with LTO, and makes it possible for the > > > compiler to select to use an instruction like fcmovb conditionally > > > based on the targeted ISA level rather than having to do a .S file > > > with hard-coded preprocessor conditionals. > > > > While this is generally good idea, there's no guarantee that a compiler > > will emit a branch-free instruction sequence like those shown above. > > I also doubt that a compiler will produce the 5 instruction sequence > > shown in my patch for src/math/i386/remquo.S which collects the FPU > > flags C0, C3 and C1 set by FPREM. > > For that you'd probably put the collection of bits inside the asm. It > still makes just a few instructions of asm, with no need for external > call ABI logic in the asm. > > > I noticed that you provide .S files for "long double" on x86-64, but > > not for "double" and "float". I therefore assume that you use the > > SSE floating-point instructions there, respectively let the compiler > > use them. > > On the x86_64 ABI, float and double arithmetic are performed in SSE > rather than in excess precision with the x87 unit. > > > Does any compiler emit branch-free instruction sequences like the > > following for Intel CPUs without SSE4.1, i.e. without ROUNDSS/ROUNDSD? > > > > .code ; Intel syntax > > ceil proc public > > extern __real@...0000000000000:real8 > > movsd xmm1, __real@...0000000000000 > > extern __real@...0000000000000:real8 > > movsd xmm2, __real@...0000000000000 > > extern __real@...0000000000000:real8 > > movsd xmm3, __real@...0000000000000 > > movsd xmm4, xmm1 > > andnpd xmm1, xmm0 > > andpd xmm4, xmm0 > > cmpltsd xmm1, xmm3 > > andpd xmm1, xmm3 > > orpd xmm1, xmm4 > > movsd xmm3, xmm0 > > addsd xmm0, xmm1 > > subsd xmm0, xmm1 > > movsd xmm1, xmm0 > > cmpltsd xmm0, xmm3 > > andpd xmm0, xmm2 > > addsd xmm0, xmm1 > > orpd xmm0, xmm4 > > ret > > ceil endp > > > > Or instruction sequences like > > > > .code ; Intel syntax > > copysign proc public > > movd rcx, xmm0 > > movd rdx, xmm1 > > shld rcx, rdx, 1 > > ror rcx, 1 > > movd xmm0, rcx > > ret > > copysign endp > > Not quite (but it might be possible to write the C in terms of shifts > instead of masks such that it does), but I also don't think it's clear > which version is better. Yours here is mildly smaller and might > perform better, but when making changes that aren't clearly better > there should be some evidence that it's actually an improvement -- > especially if it's not just improving existing arch optimizations but > adding new ones where the C was formerly used. Generally musl avoids > asm and arch-specific files as much as possible, using them only for > things that aren't representable in C or where the C is a lot larger > or slower or both. > > > .code ; Intel syntax > > fdim proc public > > movsd xmm2, xmm0 > > cmpsd xmm0, xmm1, 6 > > subsd xmm2, xmm1 > > andpd xmm0, xmm2 > > ret > > fdim endp > > Does this handle nans correctly? > > > > It also precludes x87 stack imbalance bugs like CVE-2019-14697, which > > > make me really wary of manual changes to these files. > > > > > > Would you be interested in working on converting over the files you > > > want to optimize (or even others too) to that form at the same time as > > > doing the optimizations? > > > > I don't use musl-libc; I also don't use an OS or a compiler/assembler > > which can be used to build it. > > I just stumbled upon the functions for which I sent in patches while > > searching for code which uses Intel's FPU. > > > > > It would really help with review process and with improving the overall > > > code state. > > > > If I start using musl-libc I'd be interested and rewrite these parts. > > OK. I don't mind looking at these patches further as-is, and I'll try > to continue offering constructive comments now, but it'll be after > this release cycle (hopefully wrapping that up in the next week or so) > before consideration for merging. musl 1.2.0 is already going to be a > release with big changes (time64) and I don't want to risk subtle > breakage with new changes that haven't been reviewed in detail yet or > had time for users to test. Since you guys are discussing math optimizations, here's another one: https://www.openwall.com/lists/musl/2019/11/08/1 > > > Rich
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.