public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: "Paul E. Murphy" <murphyp@linux.vnet.ibm.com>
To: "libc-alpha@sourceware.org" <libc-alpha@sourceware.org>
Cc: Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com>
Subject: [PATCHv2] Increase internal precision of ldbl-128ibm decimal printf
Date: Mon, 21 Mar 2016 22:41:00 -0000	[thread overview]
Message-ID: <56F078A2.2070602@linux.vnet.ibm.com> (raw)

Changes from V1
* Macrotize the internal precision constant
* Modify tst-sprintf3 to catch this case
* Fix my incorrect overshift of a denormal

Realistically, should we only add 1 bit of extra precision?  That
should be sufficient to know which representable number we fall
closer to.  7 is consistent with how the number is presented in
hex though.

----8<----
When the signs differ, the precision of the conversion sometimes
drops below 106 bits.  This strategy is identical to the
hexadecimal variant.

I've refactored tst-sprintf3 to enable testing a value with more
than 30 significant digits in order to demonstrate this failure
and its solution.

Additionally, this implicitly fixes a typo in the shift
quantities when subtracting from the high mantissa to compute
the difference.

2016-03-17  Paul E. Murphy  <murphyp@linux.vnet.ibm.com>

	* stdio-common/tst-sprintf3.c (TEST_N): Refactor
	TEST to take significant digits as second parameter.
	(TEST): Redefine in terms of TEST_N taking 30
	significant digits.
	(do_test): Add test case to demonstrate precision
	failure in the ldbl-128ibm printf.
	* sysdeps/ieee754/ldbl-128ibm/ldbl2pm.c:
	(__mpn_extract_long_double): Carry 7 extra intermediate
	bits of precision to aide computing difference when
	signs differ.
---
 stdio-common/tst-sprintf3.c            | 14 ++++++++++----
 sysdeps/ieee754/ldbl-128ibm/ldbl2mpn.c | 25 ++++++++++++++++++-------
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/stdio-common/tst-sprintf3.c b/stdio-common/tst-sprintf3.c
index cffd1b6..dad216b 100644
--- a/stdio-common/tst-sprintf3.c
+++ b/stdio-common/tst-sprintf3.c
@@ -38,11 +38,11 @@ do_test (void)
 # define COMPARE_LDBL(u, v) ((u).l == (v).l)
 #endif
 
-#define TEST(val) \
+#define TEST_N(val, n) \
   do									   \
     {									   \
       u.l = (val);							   \
-      snprintf (buf, sizeof buf, "%.30LgL", u.l);			   \
+      snprintf (buf, sizeof buf, "%." #n "LgL", u.l);			   \
       if (strcmp (buf, #val) != 0)					   \
 	{								   \
 	  printf ("Error on line %d: %s != %s\n", __LINE__, buf, #val);	   \
@@ -50,19 +50,25 @@ do_test (void)
 	}								   \
       if (sscanf (#val, "%Lg", &v.l) != 1 || !COMPARE_LDBL (u, v))	   \
 	{								   \
-	  printf ("Error sscanf on line %d: %.30Lg != %.30Lg\n", __LINE__, \
-		  u.l, v.l);						   \
+	  printf ("Error sscanf on line %d: %." #n "Lg != %." #n "Lg\n",   \
+		  __LINE__, u.l, v.l);					   \
 	  result = 1;							   \
 	}								   \
       /* printf ("%s %Lg %016Lx %016Lx\n", #val, u.l, u.x[0], u.x[1]); */  \
     }									   \
   while (0)
 
+#define TEST(val) TEST_N (val,30)
+
 #if LDBL_MANT_DIG >= 106
 # if LDBL_MANT_DIG == 106
   TEST (2.22507385850719347803989925739e-308L);
   TEST (2.22507385850719397210554509863e-308L);
   TEST (2.22507385850720088902458687609e-308L);
+
+  /* Verify precision is not lost for long doubles
+     of the form +1.pN,-1.pM.  */
+  TEST_N (3.32306998946228968225951765070082e+35L, 34);
 # endif
   TEST (2.22507385850720138309023271733e-308L);
   TEST (2.22507385850720187715587855858e-308L);
diff --git a/sysdeps/ieee754/ldbl-128ibm/ldbl2mpn.c b/sysdeps/ieee754/ldbl-128ibm/ldbl2mpn.c
index 4f550ef..30d9bc3 100644
--- a/sysdeps/ieee754/ldbl-128ibm/ldbl2mpn.c
+++ b/sysdeps/ieee754/ldbl-128ibm/ldbl2mpn.c
@@ -28,6 +28,12 @@
    bits (106 for long double) and an integral power of two (MPN
    frexpl). */
 
+
+/* When signs differ, the actual value is the difference between the
+   significant double and the less significant double.  Sometimes a
+   bit can be lost when we borrow from the significant mantissa.  */
+#define EXTRA_INTERNAL_PRECISION (7)
+
 mp_size_t
 __mpn_extract_long_double (mp_ptr res_ptr, mp_size_t size,
 			   int *expt, int *is_neg,
@@ -45,10 +51,15 @@ __mpn_extract_long_double (mp_ptr res_ptr, mp_size_t size,
   lo = ((long long) u.d[1].ieee.mantissa0 << 32) | u.d[1].ieee.mantissa1;
   hi = ((long long) u.d[0].ieee.mantissa0 << 32) | u.d[0].ieee.mantissa1;
 
+  /* Hold 7 extra bits of precision in the mantissa.  This allows
+     the normalizing shifts below to prevent losing precision when
+     the signs differ and the exponents are sufficiently far apart.  */
+  lo <<= EXTRA_INTERNAL_PRECISION;
+
   /* If the lower double is not a denormal or zero then set the hidden
      53rd bit.  */
   if (u.d[1].ieee.exponent != 0)
-    lo |= 1ULL << 52;
+    lo |= 1ULL << (52 + EXTRA_INTERNAL_PRECISION);
   else
     lo = lo << 1;
 
@@ -72,12 +83,12 @@ __mpn_extract_long_double (mp_ptr res_ptr, mp_size_t size,
   if (u.d[0].ieee.negative != u.d[1].ieee.negative
       && lo != 0)
     {
-      lo = (1ULL << 53) - lo;
+      lo = (1ULL << (53 + EXTRA_INTERNAL_PRECISION)) - lo;
       if (hi == 0)
 	{
 	  /* we have a borrow from the hidden bit, so shift left 1.  */
-	  hi = 0x0ffffffffffffeLL | (lo >> 51);
-	  lo = 0x1fffffffffffffLL & (lo << 1);
+	  hi = 0x000ffffffffffffeLL | (lo >> (52 + EXTRA_INTERNAL_PRECISION));
+	  lo = 0x0fffffffffffffffLL & (lo << 1);
 	  (*expt)--;
 	}
       else
@@ -85,14 +96,14 @@ __mpn_extract_long_double (mp_ptr res_ptr, mp_size_t size,
     }
 #if BITS_PER_MP_LIMB == 32
   /* Combine the mantissas to be contiguous.  */
-  res_ptr[0] = lo;
-  res_ptr[1] = (hi << (53 - 32)) | (lo >> 32);
+  res_ptr[0] = lo >> EXTRA_INTERNAL_PRECISION;
+  res_ptr[1] = (hi << (53 - 32)) | (lo >> (32 + EXTRA_INTERNAL_PRECISION));
   res_ptr[2] = hi >> 11;
   res_ptr[3] = hi >> (32 + 11);
   #define N 4
 #elif BITS_PER_MP_LIMB == 64
   /* Combine the two mantissas to be contiguous.  */
-  res_ptr[0] = (hi << 53) | lo;
+  res_ptr[0] = (hi << 53) | (lo >> EXTRA_INTERNAL_PRECISION);
   res_ptr[1] = hi >> 11;
   #define N 2
 #else
-- 
2.4.3

             reply	other threads:[~2016-03-21 22:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-21 22:41 Paul E. Murphy [this message]
2016-03-21 23:29 ` Joseph Myers
2016-03-22 14:16   ` Paul E. Murphy
2016-03-30 15:14     ` [PATCHv2][Ping 1] " Paul E. Murphy
2016-03-30 23:05       ` Joseph Myers
2016-03-31 17:18         ` Paul E. Murphy

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=56F078A2.2070602@linux.vnet.ibm.com \
    --to=murphyp@linux.vnet.ibm.com \
    --cc=libc-alpha@sourceware.org \
    --cc=tuliom@linux.vnet.ibm.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).