PATCH: fix for parsing hex floats

PATCH: fix for parsing hex floats

[Ben Elliston]

This patch, authored by Ulrich Weigand, improves parsing of  hexadecimal
floating point constants on targets with various rounding modes.  Tested
with a bootstrap and regression testsuite run on powerpc-linux.  OK for
the trunk?

Ben


2007-10-31  Ulrich Weigand  <Ulrich.Weigand@de.ibm.com>

	* c-cppbuiltin.c (builtin_define_with_hex_fp_value): Depending on
	the default target rounding mode, the decimal string we get may
	convert back to a different number when read back by the parser.
	Try to remedy this.

Index: c-cppbuiltin.c
===================================================================
--- c-cppbuiltin.c      (revision 129781)
+++ c-cppbuiltin.c      (working copy)
@@ -840,7 +840,7 @@ builtin_define_with_hex_fp_value (const 
                                  const char *fp_suffix,
                                  const char *fp_cast)
 {
-  REAL_VALUE_TYPE real;
+  REAL_VALUE_TYPE real, real2;
   char dec_str[64], buf1[256], buf2[256];
 
   /* Hex values are really cool and convenient, except that they're
@@ -856,6 +856,39 @@ builtin_define_with_hex_fp_value (const 
   real_from_string (&real, hex_str);
   real_to_decimal (dec_str, &real, sizeof (dec_str), digits, 0);
 
+  /* Depending on the default target rounding mode, the decimal string
+     we get may convert back to a different number when read back by
+     the parser.  Try to remedy this.  */
+
+  real_from_string (&real2, dec_str);
+  real_convert (&real2, TYPE_MODE (type), &real2);
+  if (!real_identical (&real2, &real))
+    {
+      char *p = dec_str;
+      while (ISDIGIT (*p))
+        p++;
+      if (*p == '.')
+        {
+          p++;
+          while (ISDIGIT (p[1]))
+           p++;
+        }
+
+      if (ISDIGIT (*p))
+       {
+         while (*p == '9')
+           {
+             *p-- = '0';
+             gcc_assert (p > dec_str && ISDIGIT (*p));
+           }
+         *p = *p + 1;
+       }
+
+      real_from_string (&real2, dec_str);
+      real_convert (&real2, TYPE_MODE (type), &real2);
+    }
+  gcc_assert (real_identical (&real2, &real));
+
   /* Assemble the macro in the following fashion
      macro = fp_cast [dec_str fp_suffix] */
   sprintf (buf1, "%s%s", dec_str, fp_suffix);

Re: PATCH: fix for parsing hex floats

[Joseph S. Myers]

On Wed, 31 Oct 2007, Ben Elliston wrote:

> This patch, authored by Ulrich Weigand, improves parsing of  hexadecimal
> floating point constants on targets with various rounding modes.  Tested
> with a bootstrap and regression testsuite run on powerpc-linux.  OK for
> the trunk?

Could we have a testcase (newly added by the patch, or already in the 
testsuite) that failed before and passed after the patch on some 
identified host and target?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: PATCH: fix for parsing hex floats

[Ulrich Weigand]

Joseph Myers wrote:
> On Wed, 31 Oct 2007, Ben Elliston wrote:
> 
> > This patch, authored by Ulrich Weigand, improves parsing of  hexadecimal
> > floating point constants on targets with various rounding modes.  Tested
> > with a bootstrap and regression testsuite run on powerpc-linux.  OK for
> > the trunk?
> 
> Could we have a testcase (newly added by the patch, or already in the 
> testsuite) that failed before and passed after the patch on some 
> identified host and target?

Actually, the problem isn't visible in current mainline; it only occurs
after switching the literal parser to respect the SPU's round-towards-zero
mode as done by Trevor's patch (still under discussion):
http://gcc.gnu.org/ml/gcc-patches/2007-10/msg01459.html

With this patch, the test case below starts to fail.  The patch posted
by Ben will fix it again.

The problem is how constants like __FLT_MIN__ are parsed.  On the SPU
(like with IEEE single-precision float), __FLT_MIN__ has the value 2^-126,
i.e. sign bit is 0, (biased) exponent is 1, and mantissa is 0.

This value in decimal is about 1.175494350822287508e-38, which gets
rounded by the default logic in builtin_define_with_hex_fp_value to
1.17549435e-38F.  Note that is is slightly smaller than the real
value.

When this define is used is source code, the parser converts it back
to a numerical representation.  By default, the parser uses a round-to-
nearest mode, so it recomputes the correct 2^-126 value.

However, once the parser uses round-to-zero, this string will actually
convert to a different result (some denormal).  This is particularly
bad on the SPU as denormal values are treated as 0 by all arithmetic
operations,  ...

Bye,
Ulrich


--- /dev/null	2007-10-11 21:52:47.626146058 +0200
+++ gcc/testsuite/gcc.target/spu/flt-round.c	2007-11-02 17:31:10.946613447 +0100
@@ -0,0 +1,25 @@
+/* { dg-do run } */
+
+/* This test case verifies that the __FLT_MIN__ constant is parsed correctly,
+   even when the parser's rounding mode is round-towards-zero.  */
+
+extern void exit (int);
+
+volatile union uf { float f; struct { unsigned int s: 1, e : 8, m : 23; } c; } uf;
+
+int main (void)
+{
+  uf.f = __FLT_MIN__;
+
+  if (uf.c.s != 0)
+    exit (1);
+
+  if (uf.c.e != 1)
+    exit (1);
+
+  if (uf.c.m != 0)
+    exit (1);
+
+  exit (0);
+}
+



-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: PATCH: fix for parsing hex floats

[Joseph S. Myers]

On Fri, 2 Nov 2007, Ulrich Weigand wrote:

> Actually, the problem isn't visible in current mainline; it only occurs
> after switching the literal parser to respect the SPU's round-towards-zero
> mode as done by Trevor's patch (still under discussion):
> http://gcc.gnu.org/ml/gcc-patches/2007-10/msg01459.html
> 
> With this patch, the test case below starts to fail.  The patch posted
> by Ben will fix it again.
> 
> The problem is how constants like __FLT_MIN__ are parsed.  On the SPU
> (like with IEEE single-precision float), __FLT_MIN__ has the value 2^-126,
> i.e. sign bit is 0, (biased) exponent is 1, and mantissa is 0.
> 
> This value in decimal is about 1.175494350822287508e-38, which gets
> rounded by the default logic in builtin_define_with_hex_fp_value to
> 1.17549435e-38F.  Note that is is slightly smaller than the real
> value.

In that case, I think that real_to_decimal should have an option to 
generate a string that rounds to the correct value after allowing for this 
target-specific rounding mode.  The C front-end changes would then be 
limited to passing the option to real_to_decimal to require it to produce 
a string that rounds correctly, and the assertion that it does indeed 
round correctly; the other logic would go in real.c (it might also be 
possible for the assertion to go there).

-- 
Joseph S. Myers
joseph@codesourcery.com