Bug 91226 - wrong propagation of non-canonical _Decimal64 and _Decimal128 constant (BID only)
Summary: wrong propagation of non-canonical _Decimal64 and _Decimal128 constant (BID o...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: 8.4
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2019-07-22 13:57 UTC by Vincent Lefèvre
Modified: 2020-01-13 22:19 UTC (History)
0 users

See Also:
Host:
Target: x86_64-*-*, i?86-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-08-06 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Vincent Lefèvre 2019-07-22 13:57:58 UTC
It appears that GCC does not propagate non-canonical _Decimal64 constants correctly, as shown by the following testcase. Tests on a Debian/unstable Linux/x86_64 machine.

#include <stdio.h>

union u
{
  _Decimal64 d64;
  unsigned long long ull;
};

#define OUT(V)                                          \
  do {                                                  \
    for (int i = 0; i < 8; i++)                         \
      printf (" %02X", ((unsigned char *) &(V))[i]);    \
    printf ("\n");                                      \
  } while (0)

#ifndef N
#define N 0
#endif

int main (void)
{
#ifdef V
  volatile
#endif
    unsigned long long i = 0x6C7386F26FC10000 - N;
  union u x;
  _Decimal64 d;

  x.ull = i;
  OUT (x.d64);
  d = x.d64;
  OUT (d);
  printf ("%a\n", (double) d);
  return 0;
}

On the following test, with -O, one can see that the memory representation has changed. The value hasn't even been canonicalized correctly, as the expected value is 0.

cventin% gcc-snapshot -Wall -Wextra tst.c -o tst -O
cventin% ./tst
 00 00 C1 6F F2 86 73 6C
 00 00 34 26 F5 6B DC 31
0x1.c6bf52634p+52

Without optimizations, no issues (and the value is 0 as expected):

cventin% gcc-snapshot -Wall -Wextra tst.c -o tst
cventin% ./tst
 00 00 C1 6F F2 86 73 6C
 00 00 C1 6F F2 86 73 6C
0x0p+0

With optimizations, but the integer constant stored in a volatile variable, the problem disappears, thus it seems to be related to the propagation of the _Decimal64 constant:

cventin% gcc-snapshot -Wall -Wextra tst.c -o tst -O -DV
cventin% ./tst
 00 00 C1 6F F2 86 73 6C
 00 00 C1 6F F2 86 73 6C
0x0p+0

The following test is the same as the first one (the issue was reproducible with -DN=0), but with a canonical _Decimal64 encoding, and the output is correct. This shows that the issue probably came from the non-canonical encoding in the first test.

cventin% gcc-snapshot -Wall -Wextra tst.c -o tst -O -DN=1
cventin% ./tst
 FF FF C0 6F F2 86 73 6C
 FF FF C0 6F F2 86 73 6C
0x1.1c37937e08p+53

Note: Due to this bug, the GNU MPFR trunk revision 13530 fails in tget_set_d64 when built with Debian's gcc-snapshot 10.0.0 20190718, trunk revision 273586. I did not see any failure with the previous GCC versions, but the above testcase fails with GCC 5.5.0, 6.5.0, 7.4.0, 8.3.0 and 9.1.0 (under Debian/unstable).
Comment 1 joseph@codesourcery.com 2019-08-06 16:45:26 UTC
This appears to be a bug in libdecnumber/bid/bid2dpd_dpd2bid.c.  
_bid_to_dpd32 checks for a too-large significand, but _bid_to_dpd64 does 
not.  Furthermore, _bid_to_dpd128 has the same bug of not checking for 
too-large significands, as shown by the following test (which outputs 0 
without optimization, which is correct, but 8e+33 with -O2, which is 
wrong).  (_bid_to_dpd128 also doesn't check at all for the case where the 
first four bits of the combination field are 1100, 1101 or 1110, in which 
case the significand is always too large and so the value is always a 
noncanonical zero.)

#include <stdio.h>

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 d;
  x.u128 = i;
  d = x.d128;
  printf ("%g\n", (double) d);
  return 0;
}
Comment 2 Joseph S. Myers 2019-08-06 16:50:11 UTC
Thus, confirmed.
Comment 3 Joseph S. Myers 2019-12-09 13:59:55 UTC
Author: jsm28
Date: Mon Dec  9 13:59:24 2019
New Revision: 279129

URL: https://gcc.gnu.org/viewcvs?rev=279129&root=gcc&view=rev
Log:
Fix libdecnumber handling of non-canonical BID significands (PR middle-end/91226).

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.

	PR middle-end/91226
libdecnumber:
	* 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:
	* 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.

Added:
    trunk/gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-1.c
    trunk/gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-2.c
    trunk/gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-3.c
    trunk/gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d128-4.c
    trunk/gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d32-1.c
    trunk/gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d32-2.c
    trunk/gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d64-1.c
    trunk/gcc/testsuite/gcc.dg/dfp/bid-non-canonical-d64-2.c
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/libdecnumber/ChangeLog
    trunk/libdecnumber/bid/bid2dpd_dpd2bid.c
Comment 4 Joseph S. Myers 2019-12-09 14:01:02 UTC
Fixed for GCC 10.  Leaving open for now for possible backports to GCC 9 and 8 branches as a wrong-code fix.
Comment 5 CVS Commits 2020-01-13 18:46:44 UTC
The releases/gcc-9 branch has been updated by Joseph Myers <jsm28@gcc.gnu.org>:

https://gcc.gnu.org/g:5b6c608019153e3dbb03e3f5f7b7a1768727f987

commit r9-8129-g5b6c608019153e3dbb03e3f5f7b7a1768727f987
Author: Joseph Myers <joseph@codesourcery.com>
Date:   Mon Jan 13 18:45:04 2020 +0000

    Fix libdecnumber handling of non-canonical BID significands (PR middle-end/91226).
    
    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.
    
    	PR middle-end/91226
    libdecnumber:
    	* 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:
    	* 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.
    
    (cherry picked from commit 0fad54f0a88160e81c3150b63c91fd9809665474)
Comment 6 CVS Commits 2020-01-13 22:16:20 UTC
The releases/gcc-8 branch has been updated by Joseph Myers <jsm28@gcc.gnu.org>:

https://gcc.gnu.org/g:a8d3c5702a422025e42b22ce860e245b50ff57a2

commit r8-9931-ga8d3c5702a422025e42b22ce860e245b50ff57a2
Author: Joseph Myers <joseph@codesourcery.com>
Date:   Mon Jan 13 22:15:01 2020 +0000

    Fix libdecnumber handling of non-canonical BID significands (PR middle-end/91226).
    
    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.
    
    	PR middle-end/91226
    libdecnumber:
    	* 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:
    	* 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.
    
    (cherry picked from commit 0fad54f0a88160e81c3150b63c91fd9809665474)
Comment 7 Joseph S. Myers 2020-01-13 22:19:37 UTC
Fixed for GCC 8.4, GCC 9.3, GCC 10.