Patchwork PING [PATCH] Fix PR libstdc++/54036, problem negating DFP NaNs

login
register
mail settings
Submitter Peter Bergner
Date Aug. 1, 2012, 11:53 p.m.
Message ID <20120801185335.1b381d653a911b8c0d5b9ec4@vnet.ibm.com>
Download mbox | patch
Permalink /patch/174621/
State New
Headers show

Comments

Peter Bergner - Aug. 1, 2012, 11:53 p.m.
On Wed, 1 Aug 2012 08:24:48 -0700 Janis Johnson wrote:
> On 08/01/2012 07:29 AM, Paolo Carlini wrote:
> > On 08/01/2012 12:46 AM, Peter Bergner wrote:
> >> I'd like to ping the following libstdc++ DFP patch that fixes PR54036:
> >>
> >>    http://gcc.gnu.org/ml/gcc-patches/2012-07/msg00959.html
> > I think the patch is essentially Ok, but I would recommend giving Janis 
> > a chance to comment (say 24/48 h).
> 
> It looks fine to me.
> 
> > Note that the patch only touches libstdc++-v3 code, thus the testcases 
> > should be added to libstdc++-v3/testsuite/decimal and the patch itself 
> > was missing a CC to libstdc++@.
> 
> I agree.

So just to be sure, like the patch below?

Also, is this ok for the 4.6 and 4.7 release branches?


Peter


libstdc++-v3/
	PR libstdc++/54036
	* include/decimal/decimal.h (_DEFINE_DECIMAL_UNARY_OP): Use _Op as
	a unary operator.

libstdc++-v3/testsuite/
	PR libstdc++/54036
	* decimal/pr54036-1.cc: New test.
	* decimal/pr54036-2.cc: Likewise.
	* decimal/pr54036-3.cc: Likewise.
Paolo Carlini - Aug. 2, 2012, 9:36 a.m.
Hi,

On 08/02/2012 01:53 AM, Peter Bergner wrote:
> So just to be sure, like the patch below?
More or less. See comments below.
> Also, is this ok for the 4.6 and 4.7 release branches?
I don't think this is a regression, thus I would say 4.7 only, to be safe.
> Peter
>
>
> libstdc++-v3/
> 	PR libstdc++/54036
> 	* include/decimal/decimal.h (_DEFINE_DECIMAL_UNARY_OP): Use _Op as
> 	a unary operator.
>
> libstdc++-v3/testsuite/
> 	PR libstdc++/54036
> 	* decimal/pr54036-1.cc: New test.
> 	* decimal/pr54036-2.cc: Likewise.
> 	* decimal/pr54036-3.cc: Likewise.
we don't have a separate ChangeLog in libstdc++-v3/testsuite/. Thus a 
single ChangeLog entry for everything.
>
> Index: libstdc++-v3/include/decimal/decimal.h
> ===================================================================
> --- libstdc++-v3/include/decimal/decimal.h	(revision 189599)
> +++ libstdc++-v3/include/decimal/decimal.h	(working copy)
> @@ -288,7 +288,7 @@
>     inline _Tp operator _Op(_Tp __rhs)		\
>     {						\
>       _Tp __tmp;					\
> -    __tmp.__setval(0 _Op __rhs.__getval());	\
> +    __tmp.__setval(_Op __rhs.__getval());	\
>       return __tmp;				\
>     }
>   
> Index: libstdc++-v3/testsuite/decimal/pr54036-1.cc
> ===================================================================
> --- libstdc++-v3/testsuite/decimal/pr54036-1.cc	(revision 0)
> +++ libstdc++-v3/testsuite/decimal/pr54036-1.cc	(revision 0)
> @@ -0,0 +1,56 @@
Copyright blurb missing. Also, you need:

// { dg-require-effective-target dfp }

Likewise for the other testcases.

> +#include <decimal/decimal>
> +using namespace std;
> +
> +decimal::decimal32
> +__attribute__ ((noinline))
> +my_nan32 (void)
> +{
> +  decimal::decimal32 z = 0;
> +  decimal::decimal32 v = z/z;
> +  return v;
> +}
> +
> +decimal::decimal32
> +__attribute__ ((noinline))
> +my_inf32 (void)
> +{
> +  decimal::decimal32 o = 1;
> +  decimal::decimal32 z = 0;
> +  decimal::decimal32 v = o/z;
> +  return v;
> +}
> +
> +int
> +main (void)
> +{
> +  decimal::decimal32 v;
> +
> +  v = my_nan32 ();
> +  if (!__builtin_isnand32 (v.__getval ()))
> +    __builtin_abort ();
> +  if (__builtin_signbitd32 (v.__getval ()))
> +    __builtin_abort ();
> +
> +  v = -v;
> +
> +  if (!__builtin_isnand32 (v.__getval ()))
> +    __builtin_abort ();
> +  if (!__builtin_signbitd32 (v.__getval ()))
> +    __builtin_abort ();
> +
> +  v = my_inf32 ();
> +  if (!__builtin_isinfd32 (v.__getval ()))
> +    __builtin_abort ();
> +  if (__builtin_signbitd32 (v.__getval ()))
> +    __builtin_abort ();
> +
> +  v = -v;
> +
> +  if (!__builtin_isinfd32 (v.__getval ()))
> +    __builtin_abort ();
> +  if (!__builtin_signbitd32 (v.__getval ()))
> +    __builtin_abort ();
> +
> +  return 0;
> +}
> +
In the library testsuite we include:

#include <testsuite_hooks.h>

and then we use VERIFY not __builtin_abort. Also, for decimal, we 
normally use:

using namespace std::decimal.

Please have the other existing decimal floating point testcases as 
reference, for these details, please try to be consistent.

Also, watch out blank lines at the end of the testcases.

Paolo.

Patch

Index: libstdc++-v3/include/decimal/decimal.h
===================================================================
--- libstdc++-v3/include/decimal/decimal.h	(revision 189599)
+++ libstdc++-v3/include/decimal/decimal.h	(working copy)
@@ -288,7 +288,7 @@ 
   inline _Tp operator _Op(_Tp __rhs)		\
   {						\
     _Tp __tmp;					\
-    __tmp.__setval(0 _Op __rhs.__getval());	\
+    __tmp.__setval(_Op __rhs.__getval());	\
     return __tmp;				\
   }
 
Index: libstdc++-v3/testsuite/decimal/pr54036-1.cc
===================================================================
--- libstdc++-v3/testsuite/decimal/pr54036-1.cc	(revision 0)
+++ libstdc++-v3/testsuite/decimal/pr54036-1.cc	(revision 0)
@@ -0,0 +1,56 @@ 
+#include <decimal/decimal>
+using namespace std;
+
+decimal::decimal32
+__attribute__ ((noinline))
+my_nan32 (void)
+{
+  decimal::decimal32 z = 0;
+  decimal::decimal32 v = z/z;
+  return v;
+}
+
+decimal::decimal32
+__attribute__ ((noinline))
+my_inf32 (void)
+{
+  decimal::decimal32 o = 1;
+  decimal::decimal32 z = 0;
+  decimal::decimal32 v = o/z;
+  return v;
+}
+
+int
+main (void)
+{
+  decimal::decimal32 v;
+
+  v = my_nan32 ();
+  if (!__builtin_isnand32 (v.__getval ()))
+    __builtin_abort ();
+  if (__builtin_signbitd32 (v.__getval ()))
+    __builtin_abort ();
+
+  v = -v;
+
+  if (!__builtin_isnand32 (v.__getval ()))
+    __builtin_abort ();
+  if (!__builtin_signbitd32 (v.__getval ()))
+    __builtin_abort ();
+
+  v = my_inf32 ();
+  if (!__builtin_isinfd32 (v.__getval ()))
+    __builtin_abort ();
+  if (__builtin_signbitd32 (v.__getval ()))
+    __builtin_abort ();
+
+  v = -v;
+
+  if (!__builtin_isinfd32 (v.__getval ()))
+    __builtin_abort ();
+  if (!__builtin_signbitd32 (v.__getval ()))
+    __builtin_abort ();
+
+  return 0;
+}
+
Index: libstdc++-v3/testsuite/decimal/pr54036-2.cc
===================================================================
--- libstdc++-v3/testsuite/decimal/pr54036-2.cc	(revision 0)
+++ libstdc++-v3/testsuite/decimal/pr54036-2.cc	(revision 0)
@@ -0,0 +1,56 @@ 
+#include <decimal/decimal>
+using namespace std;
+
+decimal::decimal64
+__attribute__ ((noinline))
+my_nan64 (void)
+{
+  decimal::decimal64 z = 0;
+  decimal::decimal64 v = z/z;
+  return v;
+}
+
+decimal::decimal64
+__attribute__ ((noinline))
+my_inf64 (void)
+{
+  decimal::decimal64 o = 1;
+  decimal::decimal64 z = 0;
+  decimal::decimal64 v = o/z;
+  return v;
+}
+
+int
+main (void)
+{
+  decimal::decimal64 v;
+
+  v = my_nan64 ();
+  if (!__builtin_isnand64 (v.__getval ()))
+    __builtin_abort ();
+  if (__builtin_signbitd64 (v.__getval ()))
+    __builtin_abort ();
+
+  v = -v;
+
+  if (!__builtin_isnand64 (v.__getval ()))
+    __builtin_abort ();
+  if (!__builtin_signbitd64 (v.__getval ()))
+    __builtin_abort ();
+
+  v = my_inf64 ();
+  if (!__builtin_isinfd64 (v.__getval ()))
+    __builtin_abort ();
+  if (__builtin_signbitd64 (v.__getval ()))
+    __builtin_abort ();
+
+  v = -v;
+
+  if (!__builtin_isinfd64 (v.__getval ()))
+    __builtin_abort ();
+  if (!__builtin_signbitd64 (v.__getval ()))
+    __builtin_abort ();
+
+  return 0;
+}
+
Index: libstdc++-v3/testsuite/decimal/pr54036-3.cc
===================================================================
--- libstdc++-v3/testsuite/decimal/pr54036-3.cc	(revision 0)
+++ libstdc++-v3/testsuite/decimal/pr54036-3.cc	(revision 0)
@@ -0,0 +1,56 @@ 
+#include <decimal/decimal>
+using namespace std;
+
+decimal::decimal128
+__attribute__ ((noinline))
+my_nan128 (void)
+{
+  decimal::decimal128 z = 0;
+  decimal::decimal128 v = z/z;
+  return v;
+}
+
+decimal::decimal128
+__attribute__ ((noinline))
+my_inf128 (void)
+{
+  decimal::decimal128 o = 1;
+  decimal::decimal128 z = 0;
+  decimal::decimal128 v = o/z;
+  return v;
+}
+
+int
+main (void)
+{
+  decimal::decimal128 v;
+
+  v = my_nan128 ();
+  if (!__builtin_isnand128 (v.__getval ()))
+    __builtin_abort ();
+  if (__builtin_signbitd128 (v.__getval ()))
+    __builtin_abort ();
+
+  v = -v;
+
+  if (!__builtin_isnand128 (v.__getval ()))
+    __builtin_abort ();
+  if (!__builtin_signbitd128 (v.__getval ()))
+    __builtin_abort ();
+
+  v = my_inf128 ();
+  if (!__builtin_isinfd128 (v.__getval ()))
+    __builtin_abort ();
+  if (__builtin_signbitd128 (v.__getval ()))
+    __builtin_abort ();
+
+  v = -v;
+
+  if (!__builtin_isinfd128 (v.__getval ()))
+    __builtin_abort ();
+  if (!__builtin_signbitd128 (v.__getval ()))
+    __builtin_abort ();
+
+  return 0;
+}
+