public inbox for libc-ports@sourceware.org
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@codesourcery.com>
To: "Joseph S. Myers" <joseph@codesourcery.com>
Cc: Thomas Schwinge <thomas@codesourcery.com>,
	<libc-alpha@sourceware.org>,	<libc-ports@sourceware.org>
Subject: Re: [BZ #15442][PATCH] MIPS/glibc: soft-fp NaN representation corrections
Date: Tue, 07 May 2013 19:39:00 -0000	[thread overview]
Message-ID: <alpine.DEB.1.10.1305072019340.26443@tp.orcam.me.uk> (raw)
In-Reply-To: <Pine.LNX.4.64.1304242111460.20942@digraph.polyomino.org.uk>

On Wed, 24 Apr 2013, Joseph S. Myers wrote:

> >  These changes touch generic code, for the most part.  They are supposed 
> > not to affect targets other than MIPS, however I think it would make sense 
> > to test them at least on a selection of other targets glibc supports.  
> > However I am not familiar enough with other targets to know how much 
> > soft-fp is used across them; also I think automatic testing may have a 
> > better value than manually poking at a function or three.  I will 
> > therefore appreciate ideas as to how to test these changes, or any other 
> > help with that indeed.
> 
> I have run powerpc-nofpu testing with this patch applied and not seen any 
> NaN-related failures, and also confirmed that the generated libc/libm 
> binaries are unchanged by the patch.

 Thanks.

> I would note that the test results would be better indicative of working 
> NaN handling if Thomas's patch 
> <http://sourceware.org/ml/libc-ports/2013-04/msg00008.html> to test sNaN 
> inputs were checked in (minus the parts that as I noted were incorrect, 
> and with appropriate conditionals, with comments referencing filed bugs, 
> for cases that don't yet pass) - which it doesn't seem to be yet.
> 
> I believe this patch should fix at least one user-visible bug regarding 
> sqrtl NaN handling (handling of input NaNs, and producing the correct sort 
> of NaN as output for negative input) - and this should show up as an 
> improvement on the test-ldouble results (although there will still be 
> plenty of failures there arising from use of fp-bit in libgcc not 
> supporting exceptions / rounding modes).

 These changes indeed cause the following progressions for New-ABI MIPS 
targets, thanks for the hint:

--- test-ldouble.out       2013-05-02 02:36:49.138155826 +0100
+++ test-ldouble.out       2013-05-03 20:01:50.576925141 +0100
@@ -382,11 +382,6 @@
 Failure: pow (-min_value, 0x1.ffffffffffffffffffffffffffffp+113) == +0: 
Exception "Underflow" not set
 Failure: pow (-min_value, max_value) == +0: Exception "Underflow" not set
 Failure: pow (2.0, -100000.0) == +0: Exception "Underflow" not set
-Failure: sqrt (qNaN) == qNaN: Exception "Invalid operation" set
-Failure: Test: sqrt (qNaN) == qNaN
-Result:
- is:         sNaN
- should be:  qNaN
 Failure: Test: sqrt (-1) == qNaN
 Result:
  is:         sNaN
@@ -10028,4 +10023,4 @@

 Test suite completed:
   11156 test cases plus 10128 tests for exception flags executed.
-  1986 errors occurred.
+  1984 errors occurred.

-- no progressions are seen for negative inputs because for those the 
result is passed from __kernel_standard (-lieee is not used by the test 
suite for some reason that I don't know; I think for the best results at 
least IEEE error handling should be tested in addition to the default of 
POSIX.1) that returns a double value and therefore has to be extended to a 
long double value with the aid of fp-bit that in turn has a bug in this 
area too (discussed here:
http://gcc.gnu.org/ml/gcc-patches/2013-04/msg00278.html).

> If that bug isn't already in Bugzilla, it should be filed there.  In any 
> case, the appropriate [BZ #N] notation should be used in the ChangeLog 
> entries (toplevel ChangeLog, and ports/ChangeLog.mips, not necessarily 
> other ports/ChangeLog.<arch> files although it's harmless to include it 
> there as well) to point to the bug, which should also be added to the 
> list of fixed bugs in NEWS as part of the fixing commit.

 I haven't found anything relevant, so this is now BZ #15442.

> The patch itself looks fine to me, but I think someone else should review 
> it as well.

 I will appreciate that too -- ping?

  Maciej

  reply	other threads:[~2013-05-07 19:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-24 17:40 [PATCH] " Maciej W. Rozycki
2013-04-24 21:24 ` Joseph S. Myers
2013-05-07 19:39   ` Maciej W. Rozycki [this message]
2013-05-13 23:19     ` [PING][BZ #15442][PATCH] " Maciej W. Rozycki
2013-05-15 15:48       ` Joseph S. Myers
2013-05-15 15:54         ` Richard Henderson
2013-05-15 18:19           ` Andreas Jaeger
2013-05-16 22:56             ` Maciej W. Rozycki

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=alpine.DEB.1.10.1305072019340.26443@tp.orcam.me.uk \
    --to=macro@codesourcery.com \
    --cc=joseph@codesourcery.com \
    --cc=libc-alpha@sourceware.org \
    --cc=libc-ports@sourceware.org \
    --cc=thomas@codesourcery.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.
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 read-only IMAP folder(s) and NNTP newsgroup(s).