Re: [PATCH] Fix DFP conversion from INTEGER_CST to REAL_CST (PR target/79487)

2017-02-15 Thread Jakub Jelinek 
On Wed, Feb 15, 2017 at 09:15:48AM +0100, Richard Biener wrote:
> >As the following testcase shows, we store decimal REAL_CSTs always in
> >_Decimal128 internal form and perform all the arithmetics on that, but
> >while
> >for arithmetics we then ensure rounding to the actual type
> >(_Decimal{32,64}
> >or for _Decimal128 no further rounding), e.g. const_binop calls
> >  inexact = real_arithmetic (, code, , );
> >  real_convert (, mode, );
> >when converting integers to _Decimal{32,64} we do nothing like that.
> >We do that only for non-decimal conversions from INTEGER_CSTs to
> >REAL_CSTs.
> >
> >The following patch fixes that.  Bootstrapped/regtested on x86_64-linux
> >(i686-linux fails to bootstrap for other reason), and on 6.x branch on
> >x86_64-linux and i686-linux.  Dominik has kindly tested it on s390x
> >(where
> >the bug has been originally reported on the float-cast-overflow-10.c
> >test).
> >
> >Ok for trunk?
> 
> OK.

Note I think it is not 100% correct because it is then suffering
from double rounding, but that is only the case for conversions from
__int128/unsigned __int128 and those are apparently not supported anyway
(things compile, but unless the conversion is optimized during compilation,
programs don't link, as the libraries don't support it).

E.g. I believe
int
main ()
{
  // 9994999
  _Decimal32 a = (((__int128_t) 0x134261629f6653ULL) << 64) | 
0x0c750eb777ffULL;
  _Decimal32 b = 9995LL;
  _Decimal32 c = 999500LL;
  // 9994499
  _Decimal32 d = (((__int128_t) 0x1342616101cf34ULL) << 64) | 
0xbc8cce9903ffULL;
  _Decimal32 e = 9994LL;
  if (a != 9.99E+34DF
  || b != 1.00E+8DF
  || c != 1.00E+14DF
  || d != 9.99E+34DF
  || e != 9.99E+7DF)
__builtin_abort ();
  return 0;
}

should pass, but it doesn't, without or with my patch, a is different.  As
only
999
   1000
are exactly representable in _Decimal32 and we do round to even, I believe
both numbers should be rounded down, but when first rounding to _Decimal128
the first number is rounded up to 9995000
and then that is rounded again up to 1000.
I've tried to tweak this, but haven't succeeded.  And I believe all of
_Decimal{32,64} arithmetics is broken similarly if evaluated by the
compiler, as decimal_real_arithmetic helpers perform all the computations
in _Decimal128 precision (so do rounding in it too) and then everything
is rounded again to _Decimal{32,64}.

Anyway, I think my patch is a significant improvement and except for the
not really supported int128 -> _Decimal{32,64} conversion it should work
fine, so I'm committing the patch now.

What I've been trying is below (incremental patch), but it didn't seem
to work well, I really don't know how decNumber works.

--- gcc/real.c  2017-02-14 21:35:35.868906203 +0100
+++ gcc/real.c  2017-02-15 12:40:22.551817716 +0100
@@ -101,7 +101,8 @@
 static void do_fix_trunc (REAL_VALUE_TYPE *, const REAL_VALUE_TYPE *);
 
 static unsigned long rtd_divmod (REAL_VALUE_TYPE *, REAL_VALUE_TYPE *);
-static void decimal_from_integer (REAL_VALUE_TYPE *);
+static void decimal_from_integer (REAL_VALUE_TYPE *,
+ const struct real_format *);
 static void decimal_integer_string (char *, const REAL_VALUE_TYPE *,
size_t);
 
@@ -2265,8 +2266,8 @@
 }
 
   if (fmt.decimal_p ())
-decimal_from_integer (r);
-  if (fmt)
+decimal_from_integer (r, fmt);
+  else if (fmt)
 real_convert (r, fmt, r);
 }
 
@@ -2320,12 +2321,12 @@
 /* Convert a real with an integral value to decimal float.  */
 
 static void
-decimal_from_integer (REAL_VALUE_TYPE *r)
+decimal_from_integer (REAL_VALUE_TYPE *r, const struct real_format *fmt)
 {
   char str[256];
 
   decimal_integer_string (str, r, sizeof (str) - 1);
-  decimal_real_from_string (r, str);
+  decimal_real_from_string (r, str, fmt);
 }
 
 /* Returns 10**2**N.  */
--- gcc/dfp.c.jj2017-01-01 12:45:38.0 +0100
+++ gcc/dfp.c   2017-02-15 12:38:49.627086419 +0100
@@ -67,11 +67,19 @@ decimal_from_decnumber (REAL_VALUE_TYPE
 /* Create decimal encoded R from string S.  */
 
 void
-decimal_real_from_string (REAL_VALUE_TYPE *r, const char *s)
+decimal_real_from_string (REAL_VALUE_TYPE *r, const char *s,
+ const real_format *fmt)
 {
   decNumber dn;
   decContext set;
-  decContextDefault (, DEC_INIT_DECIMAL128);
+  if (fmt == _quad_format)
+decContextDefault (, DEC_INIT_DECIMAL128);
+  else if (fmt == _double_format)
+decContextDefault (, DEC_INIT_DECIMAL64);
+  else if (fmt == _single_format)
+decContextDefault (, DEC_INIT_DECIMAL64);
+  else
+gcc_unreachable ();
   set.traps = 0;
 
   decNumberFromString (, s, );
@@ -82,6 +90,12 @@ decimal_real_from_string (REAL_VALUE_TYP
   decimal_from_decnumber

Re: [PATCH] Fix DFP conversion from INTEGER_CST to REAL_CST (PR target/79487)

2017-02-15 Thread Richard Biener 
On February 15, 2017 7:53:58 AM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>As the following testcase shows, we store decimal REAL_CSTs always in
>_Decimal128 internal form and perform all the arithmetics on that, but
>while
>for arithmetics we then ensure rounding to the actual type
>(_Decimal{32,64}
>or for _Decimal128 no further rounding), e.g. const_binop calls
>  inexact = real_arithmetic (, code, , );
>  real_convert (, mode, );
>when converting integers to _Decimal{32,64} we do nothing like that.
>We do that only for non-decimal conversions from INTEGER_CSTs to
>REAL_CSTs.
>
>The following patch fixes that.  Bootstrapped/regtested on x86_64-linux
>(i686-linux fails to bootstrap for other reason), and on 6.x branch on
>x86_64-linux and i686-linux.  Dominik has kindly tested it on s390x
>(where
>the bug has been originally reported on the float-cast-overflow-10.c
>test).
>
>Ok for trunk?

OK.

Richard.

>2017-02-15  Jakub Jelinek  
>
>   PR target/79487
>   * real.c (real_from_integer): Call real_convert even for decimal.
>
>   * gcc.dg/dfp/pr79487.c: New test.
>   * c-c++-common/ubsan/float-cast-overflow-8.c (TEST): Revert
>   2017-02-13 change.
>
>--- gcc/real.c.jj  2017-01-01 12:45:37.0 +0100
>+++ gcc/real.c 2017-02-14 21:35:35.868906203 +0100
>@@ -2266,7 +2266,7 @@ real_from_integer (REAL_VALUE_TYPE *r, f
> 
>   if (fmt.decimal_p ())
> decimal_from_integer (r);
>-  else if (fmt)
>+  if (fmt)
> real_convert (r, fmt, r);
> }
> 
>--- gcc/testsuite/gcc.dg/dfp/pr79487.c.jj  2017-02-14 22:42:33.137938789
>+0100
>+++ gcc/testsuite/gcc.dg/dfp/pr79487.c 2017-02-14 22:42:22.0
>+0100
>@@ -0,0 +1,16 @@
>+/* PR target/79487 */
>+/* { dg-options "-O2" } */
>+
>+int
>+main ()
>+{
>+  _Decimal32 a = (-9223372036854775807LL - 1LL); 
>+  _Decimal32 b = -9.223372E+18DF;
>+  if (b - a != 0.0DF)
>+__builtin_abort ();
>+  _Decimal64 c = (-9223372036854775807LL - 1LL); 
>+  _Decimal64 d = -9.223372036854776E+18DD;
>+  if (d - c != 0.0DD)
>+__builtin_abort ();
>+  return 0;
>+}
>---
>gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-8.c.jj2017-02-14
>00:08:33.0 +0100
>+++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-8.c   2017-02-15
>07:46:46.780778627 +0100
>@@ -8,7 +8,7 @@
> #define TEST(type1, type2) \
>   if (type1##_MIN)\
> { \
>-  volatile type2 min = type1##_MIN;   \
>+  type2 min = type1##_MIN;\
>   type2 add = -1.0;   \
>   while (1)   \
>   {   \
>@@ -28,7 +28,7 @@
>   volatile type1 tem3 = cvt_##type1##_##type2 (-1.0f);\
> } \
>   {   \
>-volatile type2 max = type1##_MAX; \
>+type2 max = type1##_MAX;  \
> type2 add = 1.0;  \
> while (1) \
>   {   \
>
>   Jakub

[PATCH] Fix DFP conversion from INTEGER_CST to REAL_CST (PR target/79487)

2017-02-14 Thread Jakub Jelinek 
Hi!

As the following testcase shows, we store decimal REAL_CSTs always in
_Decimal128 internal form and perform all the arithmetics on that, but while
for arithmetics we then ensure rounding to the actual type (_Decimal{32,64}
or for _Decimal128 no further rounding), e.g. const_binop calls
  inexact = real_arithmetic (, code, , );
  real_convert (, mode, );
when converting integers to _Decimal{32,64} we do nothing like that.
We do that only for non-decimal conversions from INTEGER_CSTs to REAL_CSTs.

The following patch fixes that.  Bootstrapped/regtested on x86_64-linux
(i686-linux fails to bootstrap for other reason), and on 6.x branch on
x86_64-linux and i686-linux.  Dominik has kindly tested it on s390x (where
the bug has been originally reported on the float-cast-overflow-10.c test).

Ok for trunk?

2017-02-15  Jakub Jelinek  

PR target/79487
* real.c (real_from_integer): Call real_convert even for decimal.

* gcc.dg/dfp/pr79487.c: New test.
* c-c++-common/ubsan/float-cast-overflow-8.c (TEST): Revert
2017-02-13 change.

--- gcc/real.c.jj   2017-01-01 12:45:37.0 +0100
+++ gcc/real.c  2017-02-14 21:35:35.868906203 +0100
@@ -2266,7 +2266,7 @@ real_from_integer (REAL_VALUE_TYPE *r, f
 
   if (fmt.decimal_p ())
 decimal_from_integer (r);
-  else if (fmt)
+  if (fmt)
 real_convert (r, fmt, r);
 }
 
--- gcc/testsuite/gcc.dg/dfp/pr79487.c.jj   2017-02-14 22:42:33.137938789 
+0100
+++ gcc/testsuite/gcc.dg/dfp/pr79487.c  2017-02-14 22:42:22.0 +0100
@@ -0,0 +1,16 @@
+/* PR target/79487 */
+/* { dg-options "-O2" } */
+
+int
+main ()
+{
+  _Decimal32 a = (-9223372036854775807LL - 1LL); 
+  _Decimal32 b = -9.223372E+18DF;
+  if (b - a != 0.0DF)
+__builtin_abort ();
+  _Decimal64 c = (-9223372036854775807LL - 1LL); 
+  _Decimal64 d = -9.223372036854776E+18DD;
+  if (d - c != 0.0DD)
+__builtin_abort ();
+  return 0;
+}
--- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-8.c.jj 2017-02-14 
00:08:33.0 +0100
+++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-8.c2017-02-15 
07:46:46.780778627 +0100
@@ -8,7 +8,7 @@
 #define TEST(type1, type2) \
   if (type1##_MIN) \
 {  \
-  volatile type2 min = type1##_MIN;\
+  type2 min = type1##_MIN; \
   type2 add = -1.0;\
   while (1)\
{   \
@@ -28,7 +28,7 @@
   volatile type1 tem3 = cvt_##type1##_##type2 (-1.0f); \
 }  \
   {\
-volatile type2 max = type1##_MAX;  \
+type2 max = type1##_MAX;   \
 type2 add = 1.0;   \
 while (1)  \
   {\

Jakub