diff mbox series

Fix libdecnumber handling of non-canonical BID significands (PR middle-end/91226)

Message ID alpine.DEB.2.21.1911292052100.31650@digraph.polyomino.org.uk
State New
Headers show
Series Fix libdecnumber handling of non-canonical BID significands (PR middle-end/91226) | expand

Commit Message

Joseph Myers Nov. 29, 2019, 8:53 p.m. UTC 
As reported in bug 91226, the libdecnumber code used on the host to
interpret DFP values in the BID encoding fails, for _Decimal64 and
_Decimal128, to check for the case where a significand is too large
and so specified in IEEE 754 to be a non-canonical encoding of the
zero significand.  This patch adds the required handling of that case,
together with tests both using -O2 (testing this host code) and -O0
(testing libgcc code, which already worked before the patch); the
tests also cover _Decimal32, which already had the required check.

In the _Decimal128 case, where the code previously completely ignored
the case where the first four bits of the combination field are 1100,
1101 or 1110, the logic for determining the correct quantum exponent
in that case is also newly added by this patch, so tests are added for
that as well (again, libgcc already handled it correctly when the
conversion was done at runtime rather than at compile time).

Bootstrapped with no regressions for x86_64-pc-linux-gnu.  OK to
commit (to trunk)?  (Note 1: we don't have a maintainer for
libdecnumber.  Note 2: as a wrong-code fix, this could be considered
later for backporting to release branches if no problems appear with
it on trunk.  Note 3: presumably binutils-gdb will pick this up at
some point through a merge of libdecnumber from the GCC repository.)

libdecnumber:
2019-11-29  Joseph Myers  <joseph@codesourcery.com>

	PR middle-end/91226
	* bid/bid2dpd_dpd2bid.c (_bid_to_dpd64): Handle non-canonical
	significands.
	(_bid_to_dpd128): Likewise.  Check for case where combination
	field starts 1100, 1101 or 1110.

gcc/testsuite:
2019-11-29  Joseph Myers  <joseph@codesourcery.com>

	PR middle-end/91226
	* gcc.dg/dfp/bid-non-canonical-d128-1.c,
	gcc.dg/dfp/bid-non-canonical-d128-2.c,
	gcc.dg/dfp/bid-non-canonical-d128-3.c,
	gcc.dg/dfp/bid-non-canonical-d128-4.c,
	gcc.dg/dfp/bid-non-canonical-d32-1.c,
	gcc.dg/dfp/bid-non-canonical-d32-2.c,
	gcc.dg/dfp/bid-non-canonical-d64-1.c,
	gcc.dg/dfp/bid-non-canonical-d64-2.c: New tests.

Comments

Joseph Myers Dec. 4, 2019, 1:11 p.m. UTC | #1 
Ping.  This patch 
<https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02686.html> is pending 
review.
Richard Sandiford Dec. 6, 2019, 11:08 a.m. UTC | #2 
Joseph Myers <joseph@codesourcery.com> writes:
> Ping.  This patch 
> <https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02686.html> is pending 
> review.

Like you say, it seems we no longer have a maintainer for this,
and I wonder how many people other than you understand it well
enough to do a meaningful technical review.

Having never worked with decimal flats, I personally can't contribute
anything useful here, but I guess from the delay that no-one else feels
they can either.  So IMO we should just go with the patch if you're
confident it's correct.

On that basis: OK by rubber stamp.

Thanks,
Richard
Jeff Law Jan. 6, 2020, 5:06 p.m. UTC | #3 
On Fri, 2019-11-29 at 20:53 +0000, Joseph Myers wrote:
> As reported in bug 91226, the libdecnumber code used on the host to
> interpret DFP values in the BID encoding fails, for _Decimal64 and
> _Decimal128, to check for the case where a significand is too large
> and so specified in IEEE 754 to be a non-canonical encoding of the
> zero significand.  This patch adds the required handling of that case,
> together with tests both using -O2 (testing this host code) and -O0
> (testing libgcc code, which already worked before the patch); the
> tests also cover _Decimal32, which already had the required check.
> 
> In the _Decimal128 case, where the code previously completely ignored
> the case where the first four bits of the combination field are 1100,
> 1101 or 1110, the logic for determining the correct quantum exponent
> in that case is also newly added by this patch, so tests are added for
> that as well (again, libgcc already handled it correctly when the
> conversion was done at runtime rather than at compile time).
> 
> Bootstrapped with no regressions for x86_64-pc-linux-gnu.  OK to
> commit (to trunk)?  (Note 1: we don't have a maintainer for
> libdecnumber.  Note 2: as a wrong-code fix, this could be considered
> later for backporting to release branches if no problems appear with
> it on trunk.  Note 3: presumably binutils-gdb will pick this up at
> some point through a merge of libdecnumber from the GCC repository.)
> 
> libdecnumber:
> 2019-11-29  Joseph Myers  <joseph@codesourcery.com>
> 
> 	PR middle-end/91226
> 	* bid/bid2dpd_dpd2bid.c (_bid_to_dpd64): Handle non-canonical
> 	significands.
> 	(_bid_to_dpd128): Likewise.  Check for case where combination
> 	field starts 1100, 1101 or 1110.
> 
> gcc/testsuite:
> 2019-11-29  Joseph Myers  <joseph@codesourcery.com>
> 
> 	PR middle-end/91226
> 	* gcc.dg/dfp/bid-non-canonical-d128-1.c,
> 	gcc.dg/dfp/bid-non-canonical-d128-2.c,
> 	gcc.dg/dfp/bid-non-canonical-d128-3.c,
> 	gcc.dg/dfp/bid-non-canonical-d128-4.c,
> 	gcc.dg/dfp/bid-non-canonical-d32-1.c,
> 	gcc.dg/dfp/bid-non-canonical-d32-2.c,
> 	gcc.dg/dfp/bid-non-canonical-d64-1.c,
> 	gcc.dg/dfp/bid-non-canonical-d64-2.c: New tests.
OK.  Your call on whether or not to backport.  Yes, binutils-gdb will
pick this up on whatever schedule works best for them.

Jeff
>
diff mbox series

Patch

Index: gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-1.c
===================================================================
--- gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-1.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-1.c	(working copy)
@@ -0,0 +1,30 @@ 
+/* Test non-canonical BID significands: _Decimal128.  Bug 91226.  */
+/* { dg-do run { target { { i?86-*-* x86_64-*-* } && lp64 } } } */
+/* { dg-options "-std=gnu2x -O2" } */
+
+extern void abort (void);
+extern void exit (int);
+
+union u
+{
+  _Decimal128 d128;
+  unsigned __int128 u128;
+};
+
+#define U128(hi, lo) (((unsigned __int128) lo) \
+		      | (((unsigned __int128) hi) << 64))
+
+int
+main (void)
+{
+  unsigned __int128 i = U128 (0x3041ed09bead87c0ULL, 0x378d8e6400000001ULL);
+  union u x;
+  _Decimal128 d128;
+  x.u128 = i;
+  d128 = x.d128;
+  volatile double d = d128;
+  if (d == 0)
+    exit (0);
+  else
+    abort ();
+}
Index: gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-2.c
===================================================================
--- gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-2.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-2.c	(working copy)
@@ -0,0 +1,42 @@ 
+/* Test non-canonical BID significands: _Decimal128, case where
+   combination field starts 11.  Bug 91226.  */
+/* { dg-do run { target { { i?86-*-* x86_64-*-* } && lp64 } } } */
+/* { dg-options "-std=gnu2x -O2" } */
+
+extern void abort (void);
+extern void exit (int);
+
+union u
+{
+  _Decimal128 d128;
+  unsigned __int128 u128;
+};
+
+#define U128(hi, lo) (((unsigned __int128) lo) \
+		      | (((unsigned __int128) hi) << 64))
+
+int
+main (void)
+{
+  unsigned __int128 i = U128 (0x6e79000000000000ULL, 0x1ULL);
+  union u x;
+  _Decimal128 d128;
+  x.u128 = i;
+  d128 = x.d128;
+  volatile double d = d128;
+  if (d != 0)
+    abort ();
+  /* The above number should have quantum exponent 1234.  */
+  _Decimal128 t1233 = 0.e1233DL, t1234 = 0.e1234DL, t1235 = 0.e1235DL;
+  _Decimal128 dx;
+  dx = d128 + t1233;
+  if (__builtin_memcmp (&dx, &t1233, 16) != 0)
+    abort ();
+  dx = d128 + t1234;
+  if (__builtin_memcmp (&dx, &t1234, 16) != 0)
+    abort ();
+  dx = d128 + t1235;
+  if (__builtin_memcmp (&dx, &t1234, 16) != 0)
+    abort ();
+  exit (0);
+}
Index: gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-3.c
===================================================================
--- gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-3.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-3.c	(working copy)
@@ -0,0 +1,5 @@ 
+/* Test non-canonical BID significands: _Decimal128.  Bug 91226.  */
+/* { dg-do run { target { { i?86-*-* x86_64-*-* } && lp64 } } } */
+/* { dg-options "-std=gnu2x -O0" } */
+
+#include "bid-non-canonical-d128-1.c"
Index: gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-4.c
===================================================================
--- gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-4.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-4.c	(working copy)
@@ -0,0 +1,6 @@ 
+/* Test non-canonical BID significands: _Decimal128, case where
+   combination field starts 11.  Bug 91226.  */
+/* { dg-do run { target { { i?86-*-* x86_64-*-* } && lp64 } } } */
+/* { dg-options "-std=gnu2x -O0" } */
+
+#include "bid-non-canonical-d128-2.c"
Index: gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d32-1.c
===================================================================
--- gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d32-1.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d32-1.c	(working copy)
@@ -0,0 +1,26 @@ 
+/* Test non-canonical BID significands: _Decimal32.  Bug 91226.  */
+/* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-std=gnu2x -O2" } */
+
+extern void abort (void);
+extern void exit (int);
+
+union u
+{
+  _Decimal32 d32;
+  unsigned int u32;
+};
+
+int
+main (void)
+{
+  union u x;
+  _Decimal32 d32;
+  x.u32 = 0x6cb89681U;
+  d32 = x.d32;
+  volatile double d = d32;
+  if (d == 0)
+    exit (0);
+  else
+    abort ();
+}
Index: gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d32-2.c
===================================================================
--- gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d32-2.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d32-2.c	(working copy)
@@ -0,0 +1,5 @@ 
+/* Test non-canonical BID significands: _Decimal32.  Bug 91226.  */
+/* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-std=gnu2x -O0" } */
+
+#include "bid-non-canonical-d32-1.c"
Index: gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d64-1.c
===================================================================
--- gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d64-1.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d64-1.c	(working copy)
@@ -0,0 +1,26 @@ 
+/* Test non-canonical BID significands: _Decimal64.  Bug 91226.  */
+/* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-std=gnu2x -O2" } */
+
+extern void abort (void);
+extern void exit (int);
+
+union u
+{
+  _Decimal64 d64;
+  unsigned long long int u64;
+};
+
+int
+main (void)
+{
+  union u x;
+  _Decimal64 d64;
+  x.u64 = 0x6c7386f26fc10001ULL;
+  d64 = x.d64;
+  volatile double d = d64;
+  if (d == 0)
+    exit (0);
+  else
+    abort ();
+}
Index: gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d64-2.c
===================================================================
--- gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d64-2.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d64-2.c	(working copy)
@@ -0,0 +1,5 @@ 
+/* Test non-canonical BID significands: _Decimal64.  Bug 91226.  */
+/* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-std=gnu2x -O0" } */
+
+#include "bid-non-canonical-d64-1.c"
Index: libdecnumber/bid/bid2dpd_dpd2bid.c
===================================================================
--- libdecnumber/bid/bid2dpd_dpd2bid.c	(revision 278845)
+++ libdecnumber/bid/bid2dpd_dpd2bid.c	(working copy)
@@ -189,6 +189,8 @@  _bid_to_dpd64 (_Decimal64 *pres, _Decimal64 *px) {
   if ((comb & 0xc00) == 0xc00) { /* G0..G1 = 11 -> exp is G2..G11 */
     exp = (comb) & 0x3ff;
     bcoeff = (x & 0x0007ffffffffffffull) | 0x0020000000000000ull;
+    if (bcoeff >= 10000000000000000ull)
+      bcoeff = 0;
   } else {
     exp = (comb >> 2) & 0x3ff;
     bcoeff = (x & 0x001fffffffffffffull);
@@ -298,9 +300,23 @@  _bid_to_dpd128 (_Decimal128 *pres, _Decimal128 *px
   if ((comb & 0x1e000) == 0x1e000) {
     res = x;
   } else { /* normal number */
-    exp = ((x.w[1] & 0x7fff000000000000ull) >> 49) & 0x3fff;
-    bcoeff.w[1] = (x.w[1] & 0x0001ffffffffffffull);
-    bcoeff.w[0] = x.w[0];
+    if ((comb & 0x18000) == 0x18000) {
+      /* Noncanonical significand (prepending 8 or 9 to any 110-bit
+	 trailing significand field produces a value above 10^34).  */
+      exp = (comb & 0x7fff) >> 1;
+      bcoeff.w[1] = 0;
+      bcoeff.w[0] = 0;
+    } else {
+      exp = ((x.w[1] & 0x7fff000000000000ull) >> 49) & 0x3fff;
+      bcoeff.w[1] = (x.w[1] & 0x0001ffffffffffffull);
+      bcoeff.w[0] = x.w[0];
+      if (bcoeff.w[1] > 0x1ed09bead87c0ull
+	  || (bcoeff.w[1] == 0x1ed09bead87c0ull
+	      && bcoeff.w[0] >= 0x378d8e6400000000ull)) {
+	bcoeff.w[1] = 0;
+	bcoeff.w[0] = 0;
+      }
+    }
     d1018 = reciprocals10_128[18];
     __mul_128x128_high (BH, bcoeff, d1018);
     amount = recip_scale[18];