[Bug middle-end/34969] New: [4.3 regression] ICE with -fipa-cp -ffast-math

[Bug middle-end/34969] New: [4.3 regression] ICE with -fipa-cp -ffast-math

[reichelt at gcc dot gnu dot org]

The following valid testcase triggers an ICE on mainline when compiled with
"gcc -O -fipa-cp -ffast-math":

===========================================
double foo(double x) { return x*x; }
double bar()         { return foo(0); }
===========================================

bug.c:2: error: edge double T.0(double)->double __builtin_pow(double, double)
has no corresponding call_stmt
bug.c:2: internal compiler error: Segmentation fault
Please submit a full bug report, [etc.]

Note the bug in the bug: verify_cgraph_node manages to print the first part of
the internal error, but crashes before "verify_cgraph_node failed" can be
printed.


-- 
           Summary: [4.3 regression] ICE with -fipa-cp -ffast-math
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code, monitored
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: reichelt at gcc dot gnu dot org


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

[Bug middle-end/34969] [4.3 regression] ICE with -fipa-cp -ffast-math

[reichelt at gcc dot gnu dot org]

-- 

reichelt at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.3.0


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

[Bug middle-end/34969] [4.3 regression] ICE with -fipa-cp -ffast-math

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2008-01-25 10:06 -------
We ICE in verify_cgraph because at the time we dump the call stmt here:

#5  0x0000000000b9d3ec in verify_cgraph_node (node=0x2af3434e1e00)
    at /space/rguenther/src/svn/trunk/gcc/cgraphunit.c:800
800                   debug_generic_stmt (e->call_stmt);
(gdb) l
795               if (!e->aux)
796                 {
797                   error ("edge %s->%s has no corresponding call_stmt",
798                          cgraph_node_name (e->caller),
799                          cgraph_node_name (e->callee));
800                   debug_generic_stmt (e->call_stmt);
801                   error_found = true;

cfun is NULL.

P2 as a broken cgraph leads to further problems and this is undiagnosed
without checking enabled.

At least until this is investigated further.  Honza?


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |ice-checking
           Priority|P3                          |P2
   Last reconfirmed|0000-00-00 00:00:00         |2008-01-25 10:06:25
               date|                            |


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

[Bug middle-end/34969] [4.3 regression] ICE with -fipa-cp -ffast-math

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |jakub at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2008-01-25 10:06:25         |2008-01-28 08:23:57
               date|                            |


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

[Bug middle-end/34969] [4.3 regression] ICE with -fipa-cp -ffast-math

[jakub at gcc dot gnu dot org]

------- Comment #2 from jakub at gcc dot gnu dot org  2008-01-28 09:52 -------
Fix:
2008-01-28  Jakub Jelinek  <jakub@redhat.com>

        PR middle-end/34969
        * tree-inline.c (fold_marked_statements): Update resp. remove
        cgraph edges if a call statement has been folded.
        * cgraphunit.c (verify_cgraph_node): Set cfun to this_cfun for
        debug_generic_stmt calls, reset it back afterwards.

        * gcc.dg/pr34969.c: New test.

--- gcc/tree-inline.c.jj        2008-01-22 15:03:23.000000000 +0100
+++ gcc/tree-inline.c   2008-01-28 10:43:17.000000000 +0100
@@ -2936,9 +2936,48 @@ fold_marked_statements (int first, struc
          if (pointer_set_contains (statements, bsi_stmt (bsi)))
            {
              tree old_stmt = bsi_stmt (bsi);
+             tree old_call = get_call_expr_in (old_stmt);
+
              if (fold_stmt (bsi_stmt_ptr (bsi)))
                {
                  update_stmt (bsi_stmt (bsi));
+                 if (old_call)
+                   {
+                     /* Update or remove corresponding cgraph edge if
something changed.  */
+                     tree new_stmt = bsi_stmt (bsi);
+                     tree new_call = get_call_expr_in (new_stmt);
+                     struct cgraph_node *node = cgraph_node (cfun->decl);
+
+                     if (old_call != new_call)
+                       {
+                         struct cgraph_edge *e = cgraph_edge (node, old_stmt);
+                         struct cgraph_edge *ne = NULL;
+                         tree new_decl;
+
+                         if (e)
+                           {
+                             if (new_call)
+                               {
+                                 new_decl = get_callee_fndecl (new_call);
+                                 if (new_decl)
+                                   {
+                                     ne = cgraph_create_edge (node,
cgraph_node (new_decl),
+                                                              new_stmt,
e->count, e->frequency,
+                                                              e->loop_nest);
+                                     ne->inline_failed = e->inline_failed;
+                                   }
+                               }
+                             cgraph_remove_edge (e);
+                           }
+                       }
+                     else if (old_stmt != new_stmt)
+                       {
+                         struct cgraph_edge *e = cgraph_edge (node, old_stmt);
+
+                         if (e)
+                           cgraph_set_call_stmt (e, new_stmt);
+                       }
+                   }
                  if (maybe_clean_or_replace_eh_stmt (old_stmt, bsi_stmt
(bsi)))
                     tree_purge_dead_eh_edges (BASIC_BLOCK (first));
                }
--- gcc/cgraphunit.c.jj 2008-01-28 09:30:03.000000000 +0100
+++ gcc/cgraphunit.c    2008-01-28 09:31:03.000000000 +0100
@@ -658,6 +658,7 @@ verify_cgraph_node (struct cgraph_node *
   struct cgraph_edge *e;
   struct cgraph_node *main_clone;
   struct function *this_cfun = DECL_STRUCT_FUNCTION (node->decl);
+  struct function *saved_cfun = cfun;
   basic_block this_block;
   block_stmt_iterator bsi;
   bool error_found = false;
@@ -666,6 +667,8 @@ verify_cgraph_node (struct cgraph_node *
     return;

   timevar_push (TV_CGRAPH_VERIFY);
+  /* debug_generic_stmt needs correct cfun */
+  set_cfun (this_cfun);
   for (e = node->callees; e; e = e->next_callee)
     if (e->aux)
       {
@@ -808,6 +811,7 @@ verify_cgraph_node (struct cgraph_node *
       dump_cgraph_node (stderr, node);
       internal_error ("verify_cgraph_node failed");
     }
+  set_cfun (saved_cfun);
   timevar_pop (TV_CGRAPH_VERIFY);
 }

--- gcc/testsuite/gcc.dg/pr34969.c.jj   2008-01-28 10:45:55.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr34969.c      2008-01-28 10:45:29.000000000 +0100
@@ -0,0 +1,15 @@
+/* PR middle-end/34969 */
+/* { dg-do compile } */
+/* { dg-options "-O -fipa-cp -ffast-math" } */
+
+double
+foo (double x)
+{
+  return x * x;
+}
+
+double
+bar (void)
+{
+  return foo (0);
+}

If any call is folded during ipa-cp, we IMHO have to update cgraph edges,
otherwise cgraph verification afterwards can't succeed.  Honza, does this make
sense or should it be done elsewhere?  Haven't bootstrapped/regtested this yet.


-- 


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

[Bug middle-end/34969] [4.3 regression] ICE with -fipa-cp -ffast-math

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2008-01-28 11:11 -------
I suppose a simpler fix for 4.3 would be to not fold call stmts here. 
Otherwise
moving the cgraph adjustment to a helper function in cgraph.c would look
nicer.


-- 


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

[Bug middle-end/34969] [4.3 regression] ICE with -fipa-cp -ffast-math

[jakub at gcc dot gnu dot org]

------- Comment #4 from jakub at gcc dot gnu dot org  2008-01-28 12:34 -------
Not folding CALL_EXPRs would introduce a regression on g++.dg/opt/devirt1.C,
after all fold_marked_statements has been added exactly to fix that regression:
http://gcc.gnu.org/ml/gcc-patches/2007-01/msg00165.html

Regarding the helper function, do you mean a wrapper around fold_stmt which
will do the cgraph edge updates?  As fold_stmt can change the whole stmt,
the helper function needs to know the old as well as new statement to do the
updates.


-- 


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

[Bug middle-end/34969] [4.3 regression] ICE with -fipa-cp -ffast-math

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2008-01-28 12:39 -------
No, I mean providing something like cgraph_update_edges_for_call_stmt (tree
old, tree new); or alternatively cgraph_remove_edge_from_call_stmt () and
cgraph_add_edge_from_call_stmt () and call those two unconditionally.


-- 


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

[Bug middle-end/34969] [4.3 regression] ICE with -fipa-cp -ffast-math

[hubicka at ucw dot cz]

------- Comment #6 from hubicka at ucw dot cz  2008-01-28 20:51 -------
Subject: Re:  [4.3 regression] ICE with -fipa-cp -ffast-math

> No, I mean providing something like cgraph_update_edges_for_call_stmt (tree
> old, tree new); or alternatively cgraph_remove_edge_from_call_stmt () and
> cgraph_add_edge_from_call_stmt () and call those two unconditionally.

My stragegy so far was to rebuild cgraph edges from scratch when needed
(that is something possibly changed).  We can probably handle that via
function local TODO flag here too.

Updating the edges across multiple passes is kind of sliperly, since we
would need to tie cgraph a lot more with gimple, pretty much as we do
for CFG.  This seems too much pie in the sky project with current
organization of trees and folders, I hope that with tuples we will have
a lot closer correspondence in between actual statements and calls here.

Since we need to have edges up to date across inliner, I guess the patch
is fine (as would be addint the TODO flag).  Thanks for looking into it!

Honza


-- 


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

[Bug middle-end/34969] [4.3 regression] ICE with -fipa-cp -ffast-math

[jakub at gcc dot gnu dot org]

------- Comment #7 from jakub at gcc dot gnu dot org  2008-01-29 23:22 -------
Subject: Bug 34969

Author: jakub
Date: Tue Jan 29 23:21:24 2008
New Revision: 131946

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131946
Log:
        PR middle-end/34969
        * cgraph.h (cgraph_update_edges_for_call_stmt): New prototype.
        * cgraph.c (cgraph_update_edges_for_call_stmt): New function.
        * tree-inline.c (fold_marked_statements): Call
        cgraph_update_edges_for_call_stmt if folding a call statement.
        * cgraphunit.c (verify_cgraph_node): Set cfun to this_cfun for
        debug_generic_stmt calls, reset it back afterwards.

        * gcc.dg/pr34969.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/pr34969.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cgraph.c
    trunk/gcc/cgraph.h
    trunk/gcc/cgraphunit.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-inline.c


-- 


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

[Bug middle-end/34969] [4.3 regression] ICE with -fipa-cp -ffast-math

[jakub at gcc dot gnu dot org]

------- Comment #8 from jakub at gcc dot gnu dot org  2008-01-29 23:22 -------
Fixed.


-- 

jakub at gcc dot gnu dot org changed:

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


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