Go patch committed: Avoid negative zero in float constants

Go patch committed: Avoid negative zero in float constants

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

This patch to the Go frontend checks for negative numbers with very
small magnitudes that will round to negative zero, and forces them to
positive zero instead.  This implements the spec clarification in
https://golang.org/cl/14727.  The test is in
https://golang.org/cl/91895.  This fixes golang.org/issue/12621.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3101 bytes --]

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 257379)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@
-36594b69b94326014c331fe50a5a345ef4f8de16
+7eebd495df915ab87926b8dd88f554674cfdacea
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc	(revision 257379)
+++ gcc/go/gofrontend/expressions.cc	(working copy)
@@ -16158,10 +16158,16 @@ Numeric_constant::set_float(Type* type,
   this->clear();
   this->classification_ = NC_FLOAT;
   this->type_ = type;
+
   // Numeric constants do not have negative zero values, so remove
   // them here.  They also don't have infinity or NaN values, but we
   // should never see them here.
-  if (mpfr_zero_p(val))
+  int bits = 0;
+  if (type != NULL
+      && type->float_type() != NULL
+      && !type->float_type()->is_abstract())
+    bits = type->float_type()->bits();
+  if (Numeric_constant::is_float_zero(val, bits))
     mpfr_init_set_ui(this->u_.float_val, 0, GMP_RNDN);
   else
     mpfr_init_set(this->u_.float_val, val, GMP_RNDN);
@@ -16175,8 +16181,50 @@ Numeric_constant::set_complex(Type* type
   this->clear();
   this->classification_ = NC_COMPLEX;
   this->type_ = type;
+
+  // Avoid negative zero as in set_float.
+  int bits = 0;
+  if (type != NULL
+      && type->complex_type() != NULL
+      && !type->complex_type()->is_abstract())
+    bits = type->complex_type()->bits() / 2;
+
+  mpfr_t real;
+  mpfr_init_set(real, mpc_realref(val), GMP_RNDN);
+  if (Numeric_constant::is_float_zero(real, bits))
+    mpfr_set_ui(real, 0, GMP_RNDN);
+
+  mpfr_t imag;
+  mpfr_init_set(imag, mpc_imagref(val), GMP_RNDN);
+  if (Numeric_constant::is_float_zero(imag, bits))
+    mpfr_set_ui(imag, 0, GMP_RNDN);
+
   mpc_init2(this->u_.complex_val, mpc_precision);
-  mpc_set(this->u_.complex_val, val, MPC_RNDNN);
+  mpc_set_fr_fr(this->u_.complex_val, real, imag, MPC_RNDNN);
+
+  mpfr_clear(real);
+  mpfr_clear(imag);
+}
+
+// Return whether VAL, at a precision of BITS, is zero.  BITS may be
+// zero in which case it is ignored.
+
+bool
+Numeric_constant::is_float_zero(const mpfr_t val, int bits)
+{
+  if (mpfr_zero_p(val))
+    return true;
+  switch (bits)
+    {
+    case 0:
+      return false;
+    case 32:
+      return mpfr_get_flt(val, GMP_RNDN) == 0;
+    case 64:
+      return mpfr_get_d(val, GMP_RNDN) == 0;
+    default:
+      go_unreachable();
+    }
 }
 
 // Get an int value.
Index: gcc/go/gofrontend/expressions.h
===================================================================
--- gcc/go/gofrontend/expressions.h	(revision 257379)
+++ gcc/go/gofrontend/expressions.h	(working copy)
@@ -4220,6 +4220,9 @@ class Numeric_constant
   bool
   check_complex_type(Complex_type*, bool, Location);
 
+  static bool
+  is_float_zero(const mpfr_t, int bits);
+
   // The kinds of constants.
   enum Classification
   {

Re: Go patch committed: Avoid negative zero in float constants

[Rainer Orth]

Hi Ian,

> This patch to the Go frontend checks for negative numbers with very
> small magnitudes that will round to negative zero, and forces them to
> positive zero instead.  This implements the spec clarification in
> https://golang.org/cl/14727.  The test is in
> https://golang.org/cl/91895.  This fixes golang.org/issue/12621.
> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> to mainline.

unfortunately, this broke bootstrap with mpfr 2.4.2, which is still the
minimum version documented in install.texi:

/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc: In static member function 'static bool Numeric_constant::is_float_zero(const __mpfr_struct*, int)':
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:16222:14: error: 'mpfr_get_flt' was not declared in this scope
       return mpfr_get_flt(val, GMP_RNDN) == 0;
              ^~~~~~~~~~~~
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:16222:14: note: suggested alternative: 'mpfr_get_ld'
       return mpfr_get_flt(val, GMP_RNDN) == 0;
              ^~~~~~~~~~~~
              mpfr_get_ld

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Go patch committed: Avoid negative zero in float constants

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

On Mon, Feb 5, 2018 at 10:47 AM, Rainer Orth
<ro@cebitec.uni-bielefeld.de> wrote:
>
>> This patch to the Go frontend checks for negative numbers with very
>> small magnitudes that will round to negative zero, and forces them to
>> positive zero instead.  This implements the spec clarification in
>> https://golang.org/cl/14727.  The test is in
>> https://golang.org/cl/91895.  This fixes golang.org/issue/12621.
>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
>> to mainline.
>
> unfortunately, this broke bootstrap with mpfr 2.4.2, which is still the
> minimum version documented in install.texi:

Bother.  Thanks for the note.  I've rolled back the patch, as follows.

Ian

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3101 bytes --]

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 257390)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@
-7eebd495df915ab87926b8dd88f554674cfdacea
+c02c71187c9794b50444e2858c582e66a3442ee8
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc	(revision 257390)
+++ gcc/go/gofrontend/expressions.cc	(working copy)
@@ -16158,16 +16158,10 @@ Numeric_constant::set_float(Type* type,
   this->clear();
   this->classification_ = NC_FLOAT;
   this->type_ = type;
-
   // Numeric constants do not have negative zero values, so remove
   // them here.  They also don't have infinity or NaN values, but we
   // should never see them here.
-  int bits = 0;
-  if (type != NULL
-      && type->float_type() != NULL
-      && !type->float_type()->is_abstract())
-    bits = type->float_type()->bits();
-  if (Numeric_constant::is_float_zero(val, bits))
+  if (mpfr_zero_p(val))
     mpfr_init_set_ui(this->u_.float_val, 0, GMP_RNDN);
   else
     mpfr_init_set(this->u_.float_val, val, GMP_RNDN);
@@ -16181,50 +16175,8 @@ Numeric_constant::set_complex(Type* type
   this->clear();
   this->classification_ = NC_COMPLEX;
   this->type_ = type;
-
-  // Avoid negative zero as in set_float.
-  int bits = 0;
-  if (type != NULL
-      && type->complex_type() != NULL
-      && !type->complex_type()->is_abstract())
-    bits = type->complex_type()->bits() / 2;
-
-  mpfr_t real;
-  mpfr_init_set(real, mpc_realref(val), GMP_RNDN);
-  if (Numeric_constant::is_float_zero(real, bits))
-    mpfr_set_ui(real, 0, GMP_RNDN);
-
-  mpfr_t imag;
-  mpfr_init_set(imag, mpc_imagref(val), GMP_RNDN);
-  if (Numeric_constant::is_float_zero(imag, bits))
-    mpfr_set_ui(imag, 0, GMP_RNDN);
-
   mpc_init2(this->u_.complex_val, mpc_precision);
-  mpc_set_fr_fr(this->u_.complex_val, real, imag, MPC_RNDNN);
-
-  mpfr_clear(real);
-  mpfr_clear(imag);
-}
-
-// Return whether VAL, at a precision of BITS, is zero.  BITS may be
-// zero in which case it is ignored.
-
-bool
-Numeric_constant::is_float_zero(const mpfr_t val, int bits)
-{
-  if (mpfr_zero_p(val))
-    return true;
-  switch (bits)
-    {
-    case 0:
-      return false;
-    case 32:
-      return mpfr_get_flt(val, GMP_RNDN) == 0;
-    case 64:
-      return mpfr_get_d(val, GMP_RNDN) == 0;
-    default:
-      go_unreachable();
-    }
+  mpc_set(this->u_.complex_val, val, MPC_RNDNN);
 }
 
 // Get an int value.
Index: gcc/go/gofrontend/expressions.h
===================================================================
--- gcc/go/gofrontend/expressions.h	(revision 257390)
+++ gcc/go/gofrontend/expressions.h	(working copy)
@@ -4220,9 +4220,6 @@ class Numeric_constant
   bool
   check_complex_type(Complex_type*, bool, Location);
 
-  static bool
-  is_float_zero(const mpfr_t, int bits);
-
   // The kinds of constants.
   enum Classification
   {

Re: Go patch committed: Avoid negative zero in float constants

[Rainer Orth]

Hi Ian,

>> unfortunately, this broke bootstrap with mpfr 2.4.2, which is still the
>> minimum version documented in install.texi:
>
> Bother.  Thanks for the note.  I've rolled back the patch, as follows.

thanks.  I'm right now experimenting to bootstrap with the current
versions.  As I'd reported before, there are testsuite failures on
Solaris (both sparc and x86)

	https://gcc.gnu.org/ml/gcc/2018-01/msg00217.html

but I don't yet know if they affect gcc.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Go patch committed: Avoid negative zero in float constants

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

On Mon, Feb 5, 2018 at 11:26 AM, Ian Lance Taylor <iant@golang.org> wrote:
> On Mon, Feb 5, 2018 at 10:47 AM, Rainer Orth
> <ro@cebitec.uni-bielefeld.de> wrote:
>>
>>> This patch to the Go frontend checks for negative numbers with very
>>> small magnitudes that will round to negative zero, and forces them to
>>> positive zero instead.  This implements the spec clarification in
>>> https://golang.org/cl/14727.  The test is in
>>> https://golang.org/cl/91895.  This fixes golang.org/issue/12621.
>>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
>>> to mainline.
>>
>> unfortunately, this broke bootstrap with mpfr 2.4.2, which is still the
>> minimum version documented in install.texi:
>
> Bother.  Thanks for the note.  I've rolled back the patch, as follows.

I have now reimplemented the patch, using only MPFR 2.4.2 API.  This
version is better anyhow.  Committed as follows after testing.

Ian

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3488 bytes --]

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 257413)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@
-1927b40e59e7c2067ecb03384b331d1be3cb5eea
+02f11a2d5cf0db2c2675c13d92bb69529f2175dd
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc	(revision 257393)
+++ gcc/go/gofrontend/expressions.cc	(working copy)
@@ -16158,10 +16158,16 @@ Numeric_constant::set_float(Type* type,
   this->clear();
   this->classification_ = NC_FLOAT;
   this->type_ = type;
+
   // Numeric constants do not have negative zero values, so remove
   // them here.  They also don't have infinity or NaN values, but we
   // should never see them here.
-  if (mpfr_zero_p(val))
+  int bits = 0;
+  if (type != NULL
+      && type->float_type() != NULL
+      && !type->float_type()->is_abstract())
+    bits = type->float_type()->bits();
+  if (Numeric_constant::is_float_neg_zero(val, bits))
     mpfr_init_set_ui(this->u_.float_val, 0, GMP_RNDN);
   else
     mpfr_init_set(this->u_.float_val, val, GMP_RNDN);
@@ -16175,8 +16181,60 @@ Numeric_constant::set_complex(Type* type
   this->clear();
   this->classification_ = NC_COMPLEX;
   this->type_ = type;
+
+  // Avoid negative zero as in set_float.
+  int bits = 0;
+  if (type != NULL
+      && type->complex_type() != NULL
+      && !type->complex_type()->is_abstract())
+    bits = type->complex_type()->bits() / 2;
+
+  mpfr_t real;
+  mpfr_init_set(real, mpc_realref(val), GMP_RNDN);
+  if (Numeric_constant::is_float_neg_zero(real, bits))
+    mpfr_set_ui(real, 0, GMP_RNDN);
+
+  mpfr_t imag;
+  mpfr_init_set(imag, mpc_imagref(val), GMP_RNDN);
+  if (Numeric_constant::is_float_neg_zero(imag, bits))
+    mpfr_set_ui(imag, 0, GMP_RNDN);
+
   mpc_init2(this->u_.complex_val, mpc_precision);
-  mpc_set(this->u_.complex_val, val, MPC_RNDNN);
+  mpc_set_fr_fr(this->u_.complex_val, real, imag, MPC_RNDNN);
+
+  mpfr_clear(real);
+  mpfr_clear(imag);
+}
+
+// Return whether VAL, at a precision of BITS, is a negative zero.
+// BITS may be zero in which case it is ignored.
+
+bool
+Numeric_constant::is_float_neg_zero(const mpfr_t val, int bits)
+{
+  if (!mpfr_signbit(val))
+    return false;
+  if (mpfr_zero_p(val))
+    return true;
+  mp_exp_t min_exp;
+  switch (bits)
+    {
+    case 0:
+      return false;
+    case 32:
+      // In a denormalized float32 the exponent is -126, and there are
+      // 24 bits of which at least the last must be 1, so the smallest
+      // representable non-zero exponent is -126 - (24 - 1) == -149.
+      min_exp = -149;
+      break;
+    case 64:
+      // Minimum exponent is -1022, there are 53 bits.
+      min_exp = -1074;
+      break;
+    default:
+      go_unreachable();
+    }
+  return mpfr_get_exp(val) < min_exp;
 }
 
 // Get an int value.
Index: gcc/go/gofrontend/expressions.h
===================================================================
--- gcc/go/gofrontend/expressions.h	(revision 257393)
+++ gcc/go/gofrontend/expressions.h	(working copy)
@@ -4220,6 +4220,9 @@ class Numeric_constant
   bool
   check_complex_type(Complex_type*, bool, Location);
 
+  static bool
+  is_float_neg_zero(const mpfr_t, int bits);
+
   // The kinds of constants.
   enum Classification
   {