Bug 28096 - [4.2 regression] fdlibm/strtod.c miscompiled at -O2
Summary: [4.2 regression] fdlibm/strtod.c miscompiled at -O2
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.2.0
: P1 normal
Target Milestone: 4.2.0
Assignee: Eric Botcazou
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2006-06-20 12:05 UTC by Jan Stein
Modified: 2006-09-30 13:35 UTC (History)
8 users (show)

See Also:
Host: i686-pc-linux-gnu
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-09-20 17:39:16


Attachments
preprocessed source (13.12 KB, text/plain)
2006-09-11 17:49 UTC, Tom Tromey
Details
valgrind report (1.91 KB, text/plain)
2006-09-11 18:01 UTC, Tom Tromey
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Stein 2006-06-20 12:05:14 UTC
It seems that converting Double/Float -> String -> Double/Float, doesn't preserve the value for small values.

The following test:

class j {
  public static void main(String [] v) throws NumberFormatException {
    System.out.println("Double.MIN_VALUE = " + Double.MIN_VALUE);
    System.out.println("Float.MIN_VALUE = " + Float.MIN_VALUE);
    System.out.println("String Double.MIN_VALUE = " +
         Double.parseDouble(Double.toString(Double.MIN_VALUE)));
    System.out.println("String Float.MIN_VALUE = " +
         Float.parseFloat(Float.toString(Float.MIN_VALUE)));
  }
}

With Cacao it generates:

Double.MIN_VALUE = 4.9E-324
Float.MIN_VALUE = 1.4E-45
String Double.MIN_VALUE = 5.0E-324
String Float.MIN_VALUE = 1.4012985E-45

With Sun JDK 1.4 it generates:

Double.MIN_VALUE = 4.9E-324
Float.MIN_VALUE = 1.4E-45
String Double.MIN_VALUE = 4.9E-324
String Float.MIN_VALUE = 1.4E-45
Comment 1 Tom Tromey 2006-06-21 14:22:34 UTC
Is there any way you could try cvs head classpath?
I fixed a bug in this code which may affect the results.
Comment 2 Jan Stein 2006-06-21 21:06:58 UTC
Subject: Re:  parseFloat and parseDouble faulty on small numbers.


I checked out the latest from SVN today. But the result was worse.  
The Double result now changed sign:

% /usr/local/bin/gij -version
java version "1.4.2"
gij (GNU libgcj) version 4.2.0 20060621 (experimental)

Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There  
is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR  
PURPOSE.

% /usr/local/bin/gij j
Double.MIN_VALUE = 4.9E-324
Float.MIN_VALUE = 1.4E-45
String Double.MIN_VALUE = -1.060997895E-314
String Float.MIN_VALUE = 1.4012985E-45

      /Jan

On 21 jun 2006, at 16.22, tromey at gcc dot gnu dot org wrote:

>
>
> ------- Comment #1 from tromey at gcc dot gnu dot org  2006-06-21  
> 14:22 -------
> Is there any way you could try cvs head classpath?
> I fixed a bug in this code which may affect the results.
>
>
> -- 
>
> tromey at gcc dot gnu dot org changed:
>
>            What    |Removed                     |Added
> ---------------------------------------------------------------------- 
> ------
>                  CC|                            |tromey at gcc dot  
> gnu dot
>                    |                            |org
>              Status|UNCONFIRMED                 |NEW
>      Ever Confirmed|0                           |1
>    Last reconfirmed|0000-00-00 00:00:00         |2006-06-21 14:22:34
>                date|                            |
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28096
>
> ------- You are receiving this mail because: -------
> You reported the bug, or are watching the reporter.

Comment 3 Jan Stein 2006-06-22 07:32:58 UTC
Subject: Re:  parseFloat and parseDouble faulty on small numbers.


Just to clarify one thing. I checked out the gcc tree (rev 114881)  
and built that. Was your fix included in that?

    /Jan

On 21 jun 2006, at 23.06, Jan Stein wrote:

>
> I checked out the latest from SVN today. But the result was worse.  
> The Double result now changed sign:
>
> % /usr/local/bin/gij -version
> java version "1.4.2"
> gij (GNU libgcj) version 4.2.0 20060621 (experimental)
>
> Copyright (C) 2006 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.   
> There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR  
> PURPOSE.
>
> % /usr/local/bin/gij j
> Double.MIN_VALUE = 4.9E-324
> Float.MIN_VALUE = 1.4E-45
> String Double.MIN_VALUE = -1.060997895E-314
> String Float.MIN_VALUE = 1.4012985E-45
>
>      /Jan
>
> On 21 jun 2006, at 16.22, tromey at gcc dot gnu dot org wrote:
>
>>
>>
>> ------- Comment #1 from tromey at gcc dot gnu dot org  2006-06-21  
>> 14:22 -------
>> Is there any way you could try cvs head classpath?
>> I fixed a bug in this code which may affect the results.
>>
>>
>> -- 
>>
>> tromey at gcc dot gnu dot org changed:
>>
>>            What    |Removed                     |Added
>> --------------------------------------------------------------------- 
>> -------
>>                  CC|                            |tromey at gcc dot  
>> gnu dot
>>                    |                            |org
>>              Status|UNCONFIRMED                 |NEW
>>      Ever Confirmed|0                           |1
>>    Last reconfirmed|0000-00-00 00:00:00         |2006-06-21 14:22:34
>>                date|                            |
>>
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28096
>>
>> ------- You are receiving this mail because: -------
>> You reported the bug, or are watching the reporter.
>

Comment 4 philippe.laporte 2006-07-26 02:31:25 UTC
the fix was probably not included since Classpath is not instantly merged into the gcc tree
Comment 5 Hanno Meyer-Thurow 2006-08-31 11:29:58 UTC
I had the same results as Jan on x86/pentium4. See:
http://gcc.gnu.org/ml/java/2006-08/msg00127.html

I rebuilt gcj (20060826) with the sse instruction set (-msse2 -mfpmath=sse) and this is my result now:

$ gij j
Double.MIN_VALUE = 5.0E-324
Float.MIN_VALUE = 1.4012985E-45
String Double.MIN_VALUE = 5.0E-324
String Float.MIN_VALUE = 1.4012985E-45
Comment 6 Andrew Pinski 2006-09-09 06:41:59 UTC
Sounds more like precission issue :).
Comment 7 Andrew Haley 2006-09-09 17:19:56 UTC
It's definitely being miscompiled.  Here's a test case:

#include <stdio.h>
#include "/mnt/zebedee/gcc/trunk/libjava/classpath/native/fdlibm/mprec.h"

int main ()
{
  const char *s = "5e-324";
  struct _Jv_reent poo;
  memset (&poo, 0, sizeof poo);
  char *se;
  double d = _strtod_r (&poo, s, &se);
  fprintf(stdout, "%g\n", d);
  return 0;
}

$ /local/install/bin/gcc p.c -I/mnt/zebedee/gcc/trunk/libjava/inlcude -I/mnt/zebedee/gcc/trunk/libjava/classpath/native/ -I/local/trunk/obj-i686-pc-linux-gnu/i686-pc-linux-gnu/libjava/include/ -I/local/trunk/obj-i686-pc-linux-gnu/i686-pc-linux-gnu/libjava/classpath/include -lgcj
 $ LD_LIBRARY_PATH=/local/install/lib ./a.out
-1.061e-314 

Recompile strtod at -O1 and it's fine:

 $ /local/install/bin/gcc p.c -I/mnt/zebedee/gcc/trunk/libjava/inlcude -I/mnt/zebedee/gcc/trunk/libjava/classpath/native/ -I/local/trunk/obj-i686-pc-linux-gnu/i686-pc-linux-gnu/libjava/include/ -I/local/trunk/obj-i686-pc-linux-gnu/i686-pc-linux-gnu/libjava/classpath/include -lgcj /home/aph/gcc/trunk/libjava/classpath/native/fdlibm/strtod.c -O1
 $ LD_LIBRARY_PATH=/local/install/lib ./a.out
4.94066e-324
Comment 8 Andrew Pinski 2006-09-11 17:42:26 UTC
Can someone provide the preprocessed source?
Comment 9 Tom Tromey 2006-09-11 17:49:18 UTC
Created attachment 12225 [details]
preprocessed source

Added a self-contained .i file

Compile with -O2 => fails
Compile with -g => works
Comment 10 Tom Tromey 2006-09-11 18:01:11 UTC
Created attachment 12226 [details]
valgrind report
Comment 11 Tom Tromey 2006-09-11 18:06:46 UTC
I compiled with -O1 and ran valgrind; the results were clean.
Comment 12 Andrew Pinski 2006-09-11 18:18:38 UTC
-O2 -fno-gcse works.
Comment 13 Danny Smith 2006-09-11 19:47:55 UTC
In my sources for David Gay's gdtoa implemntation it say this:
/* On a machine with IEEE extended-precision registers, it is
 * necessary to specify double-precision (53-bit) rounding precision
 * before invoking strtod or dtoa.  If the machine uses (the equivalent
 * of) Intel 80x87 arithmetic, the call
 *	_control87(PC_53, MCW_PC);
 * does this with many compilers.  Whether this or another call is
 * appropriate depends on the compiler; for this to work, it may be
 * necessary to #include "float.h" or another system-dependent header
 * file.
 */


There is a variant of strtod in the gdtoa file strtodnrp.c:
/* This is a variant of strtod that works on Intel ia32 systems */
/* with the default extended-precision arithmetic -- it does not */
/* require setting the precision control to 53 bits.  */
Comment 14 Eric Botcazou 2006-09-13 08:32:24 UTC
Please indicate whether it's a regression from earlier versions of GCC.
Comment 15 Andrew Haley 2006-09-14 08:27:36 UTC
I don't think this bug has anything to do with excess precision.  Valgrind shows that, when compiled at -O2, we are reading from uninitialized memory.
Comment 16 Tom Tromey 2006-09-20 16:57:14 UTC
Yes, this is a regression.
It works fine with -O2 with my system compiler (FC5 gcc, based on gcc 4.1).
It also works fine with -O2 using my gcc 4.1 build.
It fails with svn head.
Comment 17 Eric Botcazou 2006-09-20 17:38:26 UTC
> Yes, this is a regression.
> It works fine with -O2 with my system compiler (FC5 gcc, based on gcc 4.1).
> It also works fine with -O2 using my gcc 4.1 build.
> It fails with svn head.

Thanks for the info.
Comment 18 Eric Botcazou 2006-09-20 17:39:16 UTC
Investigating.
Comment 19 Eric Botcazou 2006-09-29 20:40:44 UTC
> I don't think this bug has anything to do with excess precision.  Valgrind
> shows that, when compiled at -O2, we are reading from uninitialized memory.

Confirmed:

	andl	-144(%ebp), %ebx  <--- bogus value in %ebx
	movl	%eax, -144(%ebp)

Swap the lines and all works fine.
Comment 20 Andrew Pinski 2006-09-30 08:29:52 UTC
(In reply to comment #19)
> Confirmed:
> 
>         andl    -144(%ebp), %ebx  <--- bogus value in %ebx
>         movl    %eax, -144(%ebp)

This smells like an aliasing issue.
Comment 21 Eric Botcazou 2006-09-30 08:50:20 UTC
> This smells like an aliasing issue.

But it's not, it's if-conversion.
Comment 22 Eric Botcazou 2006-09-30 13:31:38 UTC
Subject: Bug 28096

Author: ebotcazou
Date: Sat Sep 30 13:31:29 2006
New Revision: 117331

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=117331
Log:
	PR rtl-optimization/28096
	* ifcvt.c (check_cond_move_block): Return FALSE if the source of an
	assignment has already been used as a destination earlier in the
	block.


Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/20060930-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ifcvt.c
    trunk/gcc/testsuite/ChangeLog

Comment 23 Eric Botcazou 2006-09-30 13:35:28 UTC
Should work now.