Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210811082306.GC37904@port70.net>
Date: Wed, 11 Aug 2021 10:23:06 +0200
From: Szabolcs Nagy <nsz@...t70.net>
To: Stefan Kanthak <stefan.kanthak@...go.de>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] Properly simplified nextafter()

* Stefan Kanthak <stefan.kanthak@...go.de> [2021-08-11 00:53:37 +0200]:
> Szabolcs Nagy <nsz@...t70.net> wrote:
> > it's target dependent if float compares are fast or not.
> 
> It's also target dependent whether the floating-point registers
> can be accessed by integer instructions, or need to be copied:
> some win, some loose!

as Rich said, fp values are moved to int regs no matter what,
but float cmp can be avoided.

> Just let the compiler/optimizer do its job!

compilers cannot do this kind of refactoring.
(if they could, your change would not be needed)

> > (the i386 machine where i originally tested this preferred int
> > cmp and float cmp was very slow in the subnormal range and
> > iirc it also raises the non-standard input denormal exception,
> > which is fine i guess.
> 
> This exception resp. the (sticky) flag is explicitly raised/set
> in the part following the patch.

that is raised if the result is subnormal, but not if y is.
but we don't care about x86 specific denormal exception
only about standard underflow, so fcmp is fine.

> > of course soft float abis much prefer int cmp so your code is
> > likely much slower and bigger there).
> 
> 0. Doesn't musl provide target specific routines for targets with
>    soft FP?

no.

> 
> 1. If not: the compiler knows the target ABI and SHOULD generate
>    the proper integer comparisions there.

it can't, you get extern calls with register spills etc.

> > but i'm not against the change, it is likely better on modern
> > machines. did you try to benchmark it? or check the code size?
> 
> I STILL don't run a system supported by musl.

then how did you test it?

> The code is of course smaller ... but not as small and fast as a
> proper i386 or AMD64 assembly implementation ... which I can
> post upon request.

if this is an optimization patch then give us numbers
(and add them to the commit message).

at least you can look up instruction timings if you don't
want to benchmark:

e.g. aarch64 cortex-a72 optimization guide says

fcmp: 3 latency, 1 throughput
 cmp: 1 latency, 2 throughput

i.e. 1 fcmp ~ 6 cmp in the ideal case. some of the
removed instructions can execute in parallel so i'd
expect the original code to be faster on a cortex a72
for most inputs.

but on many other cores it will be faster or you can
optimize for code size or

as Rich said it makes sense to have consistent
algorithm with nexttoward (which does not use int
representation to avoid dealing with many different
long double formats). but you have to explain why
you are doing a change.

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.