mailing list of musl libc
 help / color / mirror / code / Atom feed
From: "Stefan Kanthak" <stefan.kanthak@nexgo.de>
To: "Rich Felker" <dalias@libc.org>
Cc: <musl@lists.openwall.com>
Subject: Re: More patches for math subtree
Date: Tue, 10 Dec 2019 22:32:26 +0100	[thread overview]
Message-ID: <FAF063FA3F8F4F1B883929708874E3AA@H270> (raw)
In-Reply-To: <20191210193558.GK1666@brightrain.aerifal.cx>

"Rich Felker" <dalias@libc.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.

> 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!

> 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.

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.
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@8000000000000000:real8
        movsd   xmm1, __real@8000000000000000
        extern  __real@3ff0000000000000:real8
        movsd   xmm2, __real@3ff0000000000000
        extern  __real@4330000000000000:real8
        movsd   xmm3, __real@4330000000000000
        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

        .code   ; Intel syntax
fdim    proc    public
        movsd   xmm2, xmm0
        cmpsd   xmm0, xmm1, 6
        subsd   xmm2, xmm1
        andpd   xmm0, xmm2
        ret
fdim    endp

> 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.

regards
Stefan


  reply	other threads:[~2019-12-10 21:32 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-10 16:57 Stefan Kanthak
2019-12-10 19:35 ` Rich Felker
2019-12-10 21:32   ` Stefan Kanthak [this message]
2019-12-10 22:17     ` Rich Felker
2019-12-11  1:13       ` Rosen Penev
2019-12-11  9:53       ` Stefan Kanthak
2019-12-11 10:28         ` Szabolcs Nagy

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=FAF063FA3F8F4F1B883929708874E3AA@H270 \
    --to=stefan.kanthak@nexgo.de \
    --cc=dalias@libc.org \
    --cc=musl@lists.openwall.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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

	https://git.vuxu.org/mirror/musl/

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).