Bug 34969 - [4.3 regression] ICE with -fipa-cp -ffast-math
Summary: [4.3 regression] ICE with -fipa-cp -ffast-math
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.3.0
: P2 normal
Target Milestone: 4.3.0
Assignee: Jakub Jelinek
URL:
Keywords: ice-checking, ice-on-valid-code, monitored
Depends on:
Blocks:
 
Reported: 2008-01-25 07:50 UTC by Volker Reichelt
Modified: 2008-01-29 23:22 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-01-28 08:23:57


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Volker Reichelt 2008-01-25 07:50:41 UTC
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.
Comment 1 Richard Biener 2008-01-25 10:06:25 UTC
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?
Comment 2 Jakub Jelinek 2008-01-28 09:52:20 UTC
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.
Comment 3 Richard Biener 2008-01-28 11:11:07 UTC
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.
Comment 4 Jakub Jelinek 2008-01-28 12:34:57 UTC
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.
Comment 5 Richard Biener 2008-01-28 12:39:28 UTC
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.
Comment 6 Jan Hubicka 2008-01-28 20:51:39 UTC
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
Comment 7 Jakub Jelinek 2008-01-29 23:22:09 UTC
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

Comment 8 Jakub Jelinek 2008-01-29 23:22:47 UTC
Fixed.