[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 10 Dec 2019 17:17:38 -0500
From: Rich Felker <dalias@...c.org>
To: Stefan Kanthak <stefan.kanthak@...go.de>
Cc: musl@...ts.openwall.com
Subject: Re: More patches for math subtree
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.
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.
