Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200322175325.GO11469@brightrain.aerifal.cx>
Date: Sun, 22 Mar 2020 13:53:25 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Q: dealing with missing removal of excess precision

On Sun, Mar 22, 2020 at 08:40:19PM +0300, Alexander Monakov wrote:
> On Sat, 21 Mar 2020, Rich Felker wrote:
> 
> > With the fixups mentioned (included in attached patches), i386 and
> > x86_64 are passing libc-test and seem fine. OK if I merge them
> > (rebasing the fixups in as fixups)? With s/x86/x87/ naming?
> 
> Ack for x87 naming, fabsf fix and sqrt +0x300 fix.
> 
> Nack for sqrt 'unsigned short' fix, I recommend to consider
> 'unsigned fpsr = 0' and "+a" constraint, the resulting assembly is
> much better (GCC doesn't seem to do a good job optimizing the 'unsigned short'
> variant at all).

Sorry, I forgot to disassemble and check after making that change, and
indeed it is very bad.

How about just leaving it as-is? The value is masked such that upper
bits don't matter, and "whatever the register happened to contain
before" is a valid albeit ugly output from inline asm -- it doesn't
admit any compiler transformations that would cause inconsistent value
to be observed or other problems.

> I also would like to see i386/sqrtf.c to explain why double rounding
> is safe, either as a comment (my preference) or in the commit message.
> I would write something like the following:
> 
>   The long double result has sufficient precision so that second rounding
>   to float still keeps the returned value correctly rounded, see
>   Pierre Roux, "Innocuous Double Rounding of Basic Arithmetic Operations".

I'm fine with adding that as a comment.

> (this paper elaborates the earlier [currently paywalled] paper by Figueroa)
> 
> Actually, may I ask why the initial commit did not mention that it relies
> on this nontrivial property?

The need for fixup of double was only realized later, in commit
809556e60a3359f646946879dd94c4760e5b8e84. It was discussed at the time
that no action was needed for single, but it seems since there was no
change there wasn't any mention of it in log.

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.