public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Janne Blomqvist <blomqvist.janne@gmail.com>
To: jerry DeLisle <jvdelisle@charter.net>
Cc: gfortran <fortran@gcc.gnu.org>,
	gcc patches <gcc-patches@gcc.gnu.org>,
		Thomas Henlich <thenlich@users.sourceforge.net>
Subject: Re: [patch, libgfortran] PR48906 Wrong rounding results with -m32
Date: Sat, 11 Jun 2011 08:24:00 -0000	[thread overview]
Message-ID: <BANLkTim_o8bM6bJyJxYbKW7y=kJe0fZ2ug@mail.gmail.com> (raw)
In-Reply-To: <4DF25409.1080509@charter.net>

On Fri, Jun 10, 2011 at 20:27, jerry DeLisle <jvdelisle@charter.net> wrote:
> On 06/03/2011 05:51 AM, jerry DeLisle wrote:
>>
>> Hi,
>>
>> The attached patch, which includes test cases, fixes this bug by
>> eliminating the
>> code which used floating point instructions to determine the 'r' value as
>> outlined in the Fortran standard under G formatting.
>>
>> Essentially, the code now examines the d and e values to determine the
>> number of
>> digits before and after the decimal point and whether or not to display
>> the 'E'
>> exponent symbol. Adjustments are made for various corner cases, including
>> when
>> rounding has resulted in a carry. (see PR for details of the trials)
>>
>> This patch is intrusive. It results in a minor performance improvement. It
>> eliminates a bit of code.
>>
>> Regression tested on x86-64.
>>
>> OK for trunk? then later back port to 4.6 after some proving time?
>
> Attached is updated patch based on comments from Thomas Henlich.  See my
> comment #33 and subsequent comments in the PR48906 explaining the issue with
> test case fmt_g0_6.f08 which is revised by the updated patch.
>
> Regression tested on X86-64.
>
> OK for trunk. (please ;) )

Index: gcc/testsuite/gfortran.dg/char4_iunit_1.f03
===================================================================
--- gcc/testsuite/gfortran.dg/char4_iunit_1.f03	(revision 174673)
+++ gcc/testsuite/gfortran.dg/char4_iunit_1.f03	(working copy)
@@ -24,7 +24,7 @@ program char4_iunit_1
   write(string, *) .true., .false. , .true.
   if (string .ne. 4_" T F T                                    ") call abort
   write(string, *) 1.2345e-06, 4.2846e+10_8
-  if (string .ne. 4_"   1.23450002E-06   42846000000.000000      ") call abort
+  if (string .ne. 4_"  1.234500019E-06   42846000000.000000      ") call abort
   write(string, *) nan, inf


I don't agree with this; with the patch we now output 10 significant
digits, whereas 9 is sufficient for a binary->ascii->binary roundtrip.
So please retain the "reduce d by one when E editing is used" thing
for list format and G0. This is just a side effect of using 1PGw.d
format for list format and G0 in order to avoid duplicating code, but
we don't need to follow this particular craziness that is due to how
the scale factor is specified in the standard.

Otherwise it looks nice. Good job!

FWIW, as this is quite tricky code and not a regression, IMHO we
should not backport it.

-- 
Janne Blomqvist

  reply	other threads:[~2011-06-11  6:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-03 12:51 [patch, libgfortran] " jerry DeLisle
2011-06-10 17:39 ` [patch, libgfortran] PR48906 " jerry DeLisle
2011-06-11  8:24   ` Janne Blomqvist [this message]
2011-06-11  9:13     ` Thomas Henlich
2011-06-11 14:19       ` jerry DeLisle
2011-06-11 16:05         ` Thomas Henlich
2011-06-14  6:14       ` jerry DeLisle
2011-06-14  8:30         ` Thomas Henlich
2011-06-11 20:01 jvdelisle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='BANLkTim_o8bM6bJyJxYbKW7y=kJe0fZ2ug@mail.gmail.com' \
    --to=blomqvist.janne@gmail.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jvdelisle@charter.net \
    --cc=thenlich@users.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).