[libgfortran] PR48906 Wrong rounding results with -m32

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 ;) )

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.

> 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.

I hadn't noticed this, but I agree with Janne.

It should be easy to implement:

After the switch between F and E editing, we just need to shift the
decimal point and decrement the exponent. No new rounding is required,
because we keep the number of significant digits.

On 06/11/2011 12:23 AM, Thomas Henlich wrote:
>> 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.
>
> I hadn't noticed this, but I agree with Janne.
>
> It should be easy to implement:
>
> After the switch between F and E editing, we just need to shift the
> decimal point and decrement the exponent. No new rounding is required,
> because we keep the number of significant digits.
>

Our default formats for kind=4 are:

static void
set_fnode_default (st_parameter_dt *dtp, fnode *f, int length)
{
   f->format = FMT_G;
   switch (length)
     {
     case 4:
       f->u.real.w = 16;
       f->u.real.d = 9;
       f->u.real.e = 2;
       break;

This was established as solution to PR48488 where we had two choices for 
selecting the significant digits. Nine significant digits was established as a 
requirement to guarantee round trip in all cases. The char4_iunit_1.f03 test 
case was revised because after we corrected the formatting in PR48906, it 
started to fail and I observed the test case was looking for the wrong number of 
significant digits.

Based on this, I would suggest we leave it as I have it, which is correct.

Jerry

On Sat, Jun 11, 2011 at 14:41, jerry DeLisle <jvdelisle@charter.net> wrote:
> This was established as solution to PR48488 where we had two choices for
> selecting the significant digits. Nine significant digits was established as
> a requirement to guarantee round trip in all cases. The char4_iunit_1.f03
> test case was revised because after we corrected the formatting in PR48906,
> it started to fail and I observed the test case was looking for the wrong
> number of significant digits.
>
> Based on this, I would suggest we leave it as I have it, which is correct.

I'm afraid it's not.

1.23450002E-06 has nine significant digits. That's how it should be.

We don't want 1PG16.9E2 editing for list-directed and G0,
but G16.9E2 for the F editing range and 1PE16.8E2 editing for the E range.

This is to make sure the result always has nine significant digits,
whether in the F or E range.