diff mbox

[libgfortran] Pr47567 Wrong output for small absolute values with F editing

Message ID 4D6AF7F7.6080509@frontier.com
State New
Headers show

Commit Message

Jerry DeLisle Feb. 28, 2011, 1:18 a.m. UTC 
Hi all,

I am painfully aware that we have gone through a few iterations here.  Once 
again, I hope this is the last of it.

This patch does some code clean-up. I have consolidated the handling of the 
special case of F0.d in one place and moved the special case of zero values to 
after rounding has occurred.  This way, values not zero, but rounded down to 
zero are handled correctly.

Regression tested on x86-64.  The comprehensive test case is from the PR 
reporter, Thomas Henlich. Thanks Thomas for that test case.

OK for trunk?

Jerry

2011-02-27  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/47567
	* io/write_float.def (output_float): Move handling of w = 0 to after
	output rounding. Check for zero and set zero_flag accordingly. Set
	width according to zero_flag. Add better comments.

Comments

Tobias Burnus Feb. 28, 2011, 10:04 p.m. UTC | #1 
Jerry DeLisle wrote:
> Regression tested on x86-64.  The comprehensive test case is from the 
> PR reporter, Thomas Henlich. Thanks Thomas for that test case.
> OK for trunk?

OK. Thanks for the patch.

Tobias

> 2011-02-27  Jerry DeLisle <jvdelisle@gcc.gnu.org>
>
>     PR libgfortran/47567
>     * io/write_float.def (output_float): Move handling of w = 0 to after
>     output rounding. Check for zero and set zero_flag accordingly. Set
>     width according to zero_flag. Add better comments.
H.J. Lu March 1, 2011, 5:35 a.m. UTC | #2 
On Sun, Feb 27, 2011 at 5:18 PM, Jerry DeLisle <jvdelisle@frontier.com> wrote:
> Hi all,
>
> I am painfully aware that we have gone through a few iterations here.  Once
> again, I hope this is the last of it.
>
> This patch does some code clean-up. I have consolidated the handling of the
> special case of F0.d in one place and moved the special case of zero values
> to after rounding has occurred.  This way, values not zero, but rounded down
> to zero are handled correctly.
>
> Regression tested on x86-64.  The comprehensive test case is from the PR
> reporter, Thomas Henlich. Thanks Thomas for that test case.
>
> OK for trunk?
>
> Jerry
>
> 2011-02-27  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
>
>        PR libgfortran/47567
>        * io/write_float.def (output_float): Move handling of w = 0 to after
>        output rounding. Check for zero and set zero_flag accordingly. Set
>        width according to zero_flag. Add better comments.
>

It doesn't work. I opened:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47933
diff mbox

Patch

Index: write_float.def
===================================================================
--- write_float.def	(revision 170543)
+++ write_float.def	(working copy)
@@ -109,16 +109,8 @@  output_float (st_parameter_dt *dtp, const fnode *f
 
   /* Make sure zero comes out as 0.0e0.   */
   if (zero_flag)
-    {
-      e = 0;
-      if (compile_options.sign_zero != 1)
-	sign = calculate_sign (dtp, 0);
+    e = 0;
 
-      /* Handle special cases.  */
-      if (w == 0)
-	w = d + (sign != S_NONE ? 2 : 1) + (d == 0 ? 1 : 0);
-    }
-
   /* Normalize the fractional component.  */
   buffer[2] = buffer[1];
   digits = &buffer[2];
@@ -376,15 +368,21 @@  output_float (st_parameter_dt *dtp, const fnode *f
   else
     edigits = 0;
 
-  /* Zero values always output as positive, even if the value was negative
-     before rounding.  */
+  /* Scan the digits string and count the number of zeros.  If we make it
+     all the way through the loop, we know the value is zero after the
+     rounding completed above.  */
   for (i = 0; i < ndigits; i++)
     {
       if (digits[i] != '0')
 	break;
     }
+
+  /* To format properly, we need to know if the rounded result is zero and if
+     so, we set the zero_flag which may have been already set for
+     actual zero.  */
   if (i == ndigits)
     {
+      zero_flag = true;
       /* The output is zero, so set the sign according to the sign bit unless
 	 -fno-sign-zero was specified.  */
       if (compile_options.sign_zero == 1)
@@ -393,11 +391,17 @@  output_float (st_parameter_dt *dtp, const fnode *f
 	sign = calculate_sign (dtp, 0);
     }
 
-  /* Pick a field size if none was specified.  */
+  /* Pick a field size if none was specified, taking into account small
+     values that may have been rounded to zero.  */
   if (w <= 0)
     {
-      w = nbefore + nzero + nafter + (sign != S_NONE ? 2 : 1);
-      w = w == 1 ? 2 : w;
+      if (zero_flag)
+	w = d + (sign != S_NONE ? 2 : 1) + (d == 0 ? 1 : 0);
+      else
+	{
+	  w = nbefore + nzero + nafter + (sign != S_NONE ? 2 : 1);
+	  w = w == 1 ? 2 : w;
+	}
     }
   
   /* Work out how much padding is needed.  */