Bug 54498 - [4.6 Regression] incorrect code generation from g++ -O
Summary: [4.6 Regression] incorrect code generation from g++ -O
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.7.1
: P3 normal
Target Milestone: 4.7.2
Assignee: Richard Biener
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2012-09-05 21:50 UTC by stevenj
Modified: 2013-04-12 16:30 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.7.2
Known to fail:
Last reconfirmed: 2012-09-05 00:00:00


Attachments
preprocessed source exhibiting the problem (70.34 KB, application/octet-stream)
2012-09-05 21:50 UTC, stevenj
Details

Note You need to log in before you can comment on or make changes to this bug.
Description stevenj 2012-09-05 21:50:44 UTC
Created attachment 28136 [details]
preprocessed source exhibiting the problem

The attached preprocessed source (bug.ii) illustrates an apparent incorrect code generation when it is compiled with g++ -O version 4.7.1 on x86_64 (Debian GNU/Linux).

The program executes two iterations of a loop, calling a function that returns two slightly different complex numbers in the two iterations.  After the second iteration, it prints the absolute value of the difference.  The correct output (when compiled without optimization) is:

   ft (it = 0) = -491.697+887.05i
   ft (it = 1) = -491.692+887.026i
   abs(ft - prev_ft) = 0.0245153

(0.0245153 is the correct absolute difference of the two previous numbers.)  When compiled with -O, it produces:

   ft (it = 0) = -491.697+887.05i
   ft (it = 1) = -491.692+887.026i
   abs(ft - prev_ft) = 491.692

Note that the first two numbers are the same, but the absolute value of the difference is wrong.

The problem disappears if I use g++ 4.4.5, or if I make minor changes to the code; I've tried to boil it down to the minimal code that exhibits the problem.

Steven

PS. The preprocessed source is rather long only because it #includes <stdio.h> and <complex>; the program source at the end is quite short.

PPS. Some of my g++ -v output follows, indicating the g++ configuration options etcetera:

Target: x86_64-unknown-linux-gnu
Configured with: ../configure --prefix=/home/stevenj/downloads/gcc/gcc-4.7.1/OBJ/../local --disable-multilib --enable-languages=c,c++
GNU C++ (GCC) version 4.7.1 (x86_64-unknown-linux-gnu)
	compiled by GNU C version 4.7.1, GMP version 4.3.2, MPFR version 3.0.0-p3, MPC version 0.8.2
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Comment 1 Andrew Pinski 2012-09-05 22:21:58 UTC
Confirmed, here is the source unincluded:
#include <complex>
#include <cstdio>
using namespace std;

class bar_src {
 public:
  bar_src() : next(0) {}
  virtual ~bar_src() { delete next; }

  bar_src *next;
};

class foo_src : public bar_src {
 public:
  foo_src(double f, double fwidth, double s = 5.0);
  virtual ~foo_src() {}

 private:
  double freq, width, peak_time, cutoff;
};


foo_src::foo_src(double f, double fwidth, double s) {
  freq = f; width = 1/fwidth; cutoff = s*width; peak_time = cutoff;
}

complex<double> do_ft2(int i) __attribute__ ((noinline));

complex<double> do_ft2(int i) {
  return i == 0 ? complex<double>(-491.697,887.05) : complex<double>(-491.692,887.026);
}

void foo(void) {
  complex<double> prev_ft = 0.0, ft = 0.0;
  for (int i=0; i < 2; i++) {
    prev_ft = ft;
    {
      foo_src src(1.0, 1.0 / 20);
      ft = do_ft2(i);
      printf("ft (it = %d) = %g%+gi\n", i, real(ft), imag(ft));
    }
    if (i > 0)
      printf("abs(ft - prev_ft) = %g\n",
      abs(ft - prev_ft));
  }
}

int main(void) {
  foo();
  return 0;
}

--- CUT ---
The problem comes from FRE, I think due to how it handles look though copies.  It happens on more than just x86_64.
Comment 2 Richard Biener 2012-09-06 09:58:26 UTC
Let me see.  We seem to pick up

<bb 2>:
  REALPART_EXPR <prev_ft._M_value> = 0.0;

when optimizing

 <bb 7>:
   D.25613_38 = ft._M_value;
   __r$_M_value = D.25613_38;
-  D.25615_39 = MEM[(const double &)&prev_ft];
+  D.25615_39 = 0.0;

possibly not realizing that

<bb 3>:
  prev_ft = ft;

clobbers it.
Comment 3 Richard Biener 2012-09-06 10:21:36 UTC
Testing

Index: gcc/tree-ssa-alias.c
===================================================================
--- gcc/tree-ssa-alias.c        (revision 191016)
+++ gcc/tree-ssa-alias.c        (working copy)
@@ -2094,7 +2094,10 @@ walk_non_aliased_vuses (ao_ref *ref, tre
              /* Lookup succeeded.  */
              else if (res != NULL)
                break;
-             /* Translation succeeded, continue walking.  */
+             /* Translation succeeded, continue walking.  We have to
+                again visit cycles though.  */
+             if (visited)
+               bitmap_clear (visited);
            }
          vuse = gimple_vuse (def_stmt);
        }
Comment 4 H.J. Lu 2012-09-06 12:21:36 UTC
It was caused by revision 177672:

http://gcc.gnu.org/ml/gcc-cvs/2011-08/msg00686.html
Comment 5 Richard Biener 2012-09-06 13:54:34 UTC
Latent in 4.6, too.
Comment 6 Richard Biener 2012-09-06 14:47:50 UTC
Author: rguenth
Date: Thu Sep  6 14:47:42 2012
New Revision: 191030

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191030
Log:
2012-09-06  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/54498
	* tree-ssa-alias.h (get_continuation_for_phi): Add flag to
	abort when reaching an already visited region.
	* tree-ssa-alias.c (maybe_skip_until): Likewise.  And do it.
	(get_continuation_for_phi_1): Likewise.
	(walk_non_aliased_vuses): When we translated the reference,
	abort when we re-visit a region.
	* tree-ssa-pre.c (translate_vuse_through_block): Adjust.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-alias.c
    trunk/gcc/tree-ssa-alias.h
    trunk/gcc/tree-ssa-pre.c
Comment 7 Richard Biener 2012-09-06 15:20:29 UTC
Author: rguenth
Date: Thu Sep  6 15:20:24 2012
New Revision: 191031

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191031
Log:
2012-09-06  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/54498
	* tree-ssa-alias.h (get_continuation_for_phi): Add flag to
	abort when reaching an already visited region.
	* tree-ssa-alias.c (maybe_skip_until): Likewise.  And do it.
	(get_continuation_for_phi_1): Likewise.
	(walk_non_aliased_vuses): When we translated the reference,
	abort when we re-visit a region.
	* tree-ssa-pre.c (translate_vuse_through_block): Adjust.

Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-ssa-alias.c
    branches/gcc-4_7-branch/gcc/tree-ssa-alias.h
    branches/gcc-4_7-branch/gcc/tree-ssa-pre.c
Comment 8 Richard Biener 2012-09-07 08:19:52 UTC
Sofar fixed on trunk and 4.7 branch.
Comment 9 Jakub Jelinek 2013-04-12 16:30:08 UTC
The 4.6 branch has been closed, fixed in GCC 4.7.2.