[Bug c++/30980] New: [4.3 Regression] Recent complex miscompilation

[Bug c++/30980] New: [4.3 Regression] Recent complex miscompilation

[pcarlini at suse dot de]

Recently (~10 days ago), 26_numerics/complex/13450.cc started failing for
*many* targets, among which powerpc-darwin. On the latter I'm seeing a
miscompilation at -O1 and above of this reduced C++ snippet:

#include <math.h>
#include <stdio.h>

int main()
{
  __complex__ double x;

  __real__ x = -1.0;
  __imag__ x = 0.0;

  __complex__ double t = __builtin_clog(x);

  double tmp = 0.5 * (__imag__ t);

  printf("%g %g\n", cos(tmp), sin(tmp));
}

/////////////////

Note how the output changes at -O1 and above. Also note that the very same
snippet, as C is fine, thus I'm categorizing as C++, not as middle-end or
something else.


-- 
           Summary: [4.3 Regression] Recent complex miscompilation
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: pcarlini at suse dot de
GCC target triplet: powerpc-apple-darwin8.8.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug c++/30980] [4.3 Regression] Recent complex miscompilation

[pcarlini at suse dot de]

------- Comment #1 from pcarlini at suse dot de  2007-02-27 13:20 -------
Looking at testresults, it seems 26_numerics/complex/13450.cc started failing
on powerpc-apple-darwin8.5.0 between 122031 and 122067.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2007-02-27 13:27 -------
As I cannot reproduce this on any linux target I guess the following change did
it:

+2007-02-16  Geoffrey Keating  <geoffk@apple.com>
+
+       * config/darwin.h (LINK_SPEC): Always pass -macosx_version_min
+       to linker.
+       (DARWIN_EXTRA_SPECS): Add %(darwin_minversion).
+       * config/rs6000/darwin.h (SUBTARGET_OVERRIDE_OPTIONS): Just call
+       darwin_rs6000_override_options.
+       (C_COMMON_OVERRIDE_OPTIONS): Expect
+       darwin_macosx_version_min to be non-NULL always.
+       (TARGET_C99_FUNCTIONS): Likewise.
+       (CC1_SPEC): Always pass -mmacosx-version-min to cc1*.
+       (DARWIN_MINVERSION_SPEC): New.
+       * config/rs6000/rs6000.c (darwin_rs6000_override_options): New.
+       * config/i386/darwin.h (CC1_SPEC): Always pass -mmacosx-version-min
+       to cc1*.  
+       (DARWIN_MINVERSION_SPEC): New.
+       * config/darwin.opt (mmacosx-version-min): Initialize to non-NULL
+       value.
+       * config/darwin-c.c (darwin_cpp_builtins): Expect
+       darwin_macosx_version_min to be non-NULL always.
+
+       * config/rs6000/rs6000.c: Clean up trailing whitespace.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |geoffk at gcc dot gnu dot
                   |                            |org
          Component|c++                         |target


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[pcarlini at suse dot de]

------- Comment #3 from pcarlini at suse dot de  2007-02-27 13:30 -------
Yes, I can confirm it cannot be reproduced on linux targets. Anyway, we can
easily nail down between 122051 and 122067 (from testresults).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[pcarlini at suse dot de]

------- Comment #4 from pcarlini at suse dot de  2007-02-27 13:39 -------
Note, however, that per Kaveh' reports, around the same dates (between
2007-02-14 and 2007-02-19) it started failing also on sparc-sun-solaris2.10...


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[pcarlini at suse dot de]

------- Comment #5 from pcarlini at suse dot de  2007-02-27 13:41 -------
I'm adding Eric too, maybe he wants to investigate the sparc version of the
issue.


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[pinskia at gcc dot gnu dot org]

------- Comment #6 from pinskia at gcc dot gnu dot org  2007-02-27 15:42 -------
PR 30969 is related.  The reduced fortran looks like the reduced C++.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|critical                    |normal
   Target Milestone|---                         |4.3.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[pinskia at gcc dot gnu dot org]

------- Comment #7 from pinskia at gcc dot gnu dot org  2007-02-27 15:47 -------
>  I'm adding Eric too, maybe he wants to investigate the sparc version of the
> issue.

The patch which I was worried about causing a regression with respect of
calling complex functions is:
2007-02-18  Sandra Loosemore  <sandra@codesourcery.com>

        * calls.c (initialize_argument_information): Pass original EXP
        and STRUCT_VALUE_ADDR_VALUE instead of a list of arguments.  Move
        code to split complex arguments here, as part of initializing the
        ARGS array.
        (expand_call): Remove code that builds a list of arguments and
        inserts implicit arguments into it.  Instead, just count how many
        implicit arguments there will be so we can determine the size of
        the ARGS array, and let initialize_argument_information do the work.
        (split_complex_values): Delete unused function.


Now I don't know if that patch caused the sparc regression or not.




-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[geoffk at gcc dot gnu dot org]

------- Comment #8 from geoffk at gcc dot gnu dot org  2007-02-27 17:25 -------
I'm confident that my patch could not possibly have affected targets other than
Darwin, and should not have significantly affected code generation even there. 
Complex arithmetic testcases will fail for other reasons on Darwin (like PR
24577).


-- 

geoffk at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|geoffk at gcc dot gnu dot   |
                   |org                         |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[dominiq at lps dot ens dot fr]

------- Comment #9 from dominiq at lps dot ens dot fr  2007-03-11 12:19 -------
I confirm that the bug does not show with gcc. Is theis related to the
following failures in the regtest?

FAIL: gcc.dg/builtins-59.c scan-tree-dump __builtin_cexpi
FAIL: gcc.dg/builtins-59.c scan-tree-dump-not sincos
FAIL: gcc.dg/builtins-61.c scan-tree-dump cexpi
FAIL: gcc.dg/builtins-61.c scan-tree-dump sin
FAIL: gcc.dg/builtins-61.c scan-tree-dump cos
FAIL: gcc.dg/builtins-61.c scan-tree-dump return 0.0
FAIL: gcc.dg/builtins-62.c scan-tree-dump-times cexpi 3

Also the following fortran code

complex(8) :: cplx, t
real(8)    :: tmp
cplx = cmplx(-1.0,0.0,8)
t = log(cplx)
tmp = 0.5 * imag(t)
print *, cos(tmp), sin(tmp)
end

reproduces the bug.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[dominiq at lps dot ens dot fr]

------- Comment #10 from dominiq at lps dot ens dot fr  2007-03-13 12:56 -------
The problem seems to come from a broken/unavailable __builtin_cexpi,
see PR31161.

My understanding is that __builtin_cexpi and __builtin_sincos are twin
objects(?). Now I see in gcc/tree-ssa-math-opts.c:

...
static bool       
gate_cse_sincos (void)
{                   
  /* Make sure we have either sincos or cexp.  */
  return (TARGET_HAS_SINCOS
          || TARGET_C99_FUNCTIONS)
         && optimize;
}

struct tree_opt_pass pass_cse_sincos =
{
  "sincos",                             /* name */
  gate_cse_sincos,                      /* gate */
  execute_cse_sincos,                   /* execute */
  NULL,                                 /* sub */
  NULL,                                 /* next */
  0,                                    /* static_pass_number */
  0,                                    /* tv_id */
  PROP_ssa,                             /* properties_required */
  0,                                    /* properties_provided */
  0,                                    /* properties_destroyed */
  0,                                    /* todo_flags_start */
  TODO_dump_func | TODO_update_ssa | TODO_verify_ssa
    | TODO_verify_stmts,                /* todo_flags_finish */
  0                                     /* letter */
};
...

And TARGET_C99_FUNCTIONS is set to 1 on Darwin -> gate_cse_sincos returns
true even if TARGET_HAS_SINCOS is 0.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[rguenth at gcc dot gnu dot org]

------- Comment #11 from rguenth at gcc dot gnu dot org  2007-03-13 13:22 -------
Because if TARGET_C99_FUNCTIONS is set we can expand __builtin_cexpi by using
cexp().  The ICE you get in PR30969 shows that TARGET_C99_FUNCTIONS is _not_
set:

      /* We can expand via the C99 cexp function.  */
      gcc_assert (TARGET_C99_FUNCTIONS);


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[rguenth at gcc dot gnu dot org]

------- Comment #12 from rguenth at gcc dot gnu dot org  2007-03-13 13:26 -------
Now,

2007-02-05  Roger Sayle  <roger@eyesopen.com>

        * fold-const.c (fold_unary) <REAL_PART>: Test for availability of
        BUILT_IN_COS before simplifying REAL_PART(CEXPI)) to COS.
        <IMAG_PART>: Likewise, check for availability of BUILT_IN_SIN.
        * builtins.c (fold_builtin_sincos): Check for TARGET_C99_FUNCTIONS
        before canonicalizing sincos to cexpi.
        (fold_builtin_cexp): Likewise, for canonicalizing cexp to cexpi.

is too conservative in the sense that we can expand __builtin_cexpi if
either TARGET_HAS_SINCOS or TARGET_C99_FUNCTIONS.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |roger at eyesopen dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[dominiq at lps dot ens dot fr]

------- Comment #13 from dominiq at lps dot ens dot fr  2007-03-13 14:48 -------
> The ICE you get in PR30969 shows that TARGET_C99_FUNCTIONS is _not_ set:

Is there a way to test it once the building directory is gone? In
gcc/config/rs6000/darwin.h I read:

...
/* Old versions of Mac OS/Darwin don't have C99 functions available.  */
#undef TARGET_C99_FUNCTIONS
#define TARGET_C99_FUNCTIONS                                    \
  (TARGET_64BIT                                                 \
   || strverscmp (darwin_macosx_version_min, "10.3") >= 0)
...

I assumed that TARGET_C99_FUNCTIONS is set for 10.3 and above, but I may have
overlooked something else.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[dominiq at lps dot ens dot fr]

------- Comment #14 from dominiq at lps dot ens dot fr  2007-03-17 00:26 -------
Thanks to Richard Guenther, the bug seems to be fixed (hopefully in the
20070316 snapshot):
see PR31161 for details.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[dominiq at lps dot ens dot fr]

------- Comment #15 from dominiq at lps dot ens dot fr  2007-03-22 16:22 -------
Note that the drawback of the optimization replacing sin+cos by cexpi on PPC
Darwin has been dissected in PR31249.

In comment #16, I proposed a patch. Before applying it, it would be nice to
test if the other platforms affected by this bug are also affected by the bad
optimization and if the patch does not cause timing regressions on platforms
for which the optimization is working (mostly Linux).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[mmitchel at gcc dot gnu dot org]

------- Comment #16 from mmitchel at gcc dot gnu dot org  2007-06-29 18:00 -------
This problem is reported fixed; may we close this PR?


-- 

mmitchel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980

[Bug target/30980] [4.3 Regression] Recent complex miscompilation

[pcarlini at suse dot de]

------- Comment #17 from pcarlini at suse dot de  2007-06-29 18:09 -------
Great, let's close it, then.


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30980