Patchwork [PR,57294] Update symbol table references in IPA-SRA

login
register
mail settings
Submitter Martin Jambor
Date May 24, 2013, 3:13 p.m.
Message ID <20130524151324.GB27165@virgil.suse>
Download mbox | patch
Permalink /patch/246193/
State New
Headers show

Comments

Martin Jambor - May 24, 2013, 3:13 p.m.
Hi,

when modifying an indirectly recursively called function, IPA-SRA may
remove a statement for which we have already gathered references in
the symbol table and replace it with new statement(s), making the
symbol table information stale which can lead to problems like PR
57294.

The patch below adds a function to remove references relating to a
particular statement and another one to allow callers from outside of
cgraphbuild.c record references in a particular statement, allowing
the argument modification function to update them accordingly.

Bootstrapped and tested on x86_64-linux.  OK for trunk?

Thanks,

Martin


2013-05-24  Martin Jambor  <mjambor@suse.cz>

	* cgraph.h (ipa_record_stmt_references): Declare.
	* cgraphbuild.c (ipa_record_stmt_references): New function.
	(build_cgraph_edges): Use ipa_record_stmt_references.
	(rebuild_cgraph_edges): Likewise.
	(cgraph_rebuild_references): Likewise.
	* ipa-prop.c (ipa_modify_call_arguments): Discard references
	associated with the old statement and build references from the
	newly built statements.
	* ipa-ref.c (ipa_remove_stmt_references): New function.
	* ipa-ref.h (ipa_remove_stmt_references): Declare.

testsuite/
	* gcc.dg/ipa/pr57294.c: New test.
Jan Hubicka - May 24, 2013, 3:19 p.m.
> 2013-05-24  Martin Jambor  <mjambor@suse.cz>
> 
> 	* cgraph.h (ipa_record_stmt_references): Declare.
> 	* cgraphbuild.c (ipa_record_stmt_references): New function.
> 	(build_cgraph_edges): Use ipa_record_stmt_references.
> 	(rebuild_cgraph_edges): Likewise.
> 	(cgraph_rebuild_references): Likewise.
> 	* ipa-prop.c (ipa_modify_call_arguments): Discard references
> 	associated with the old statement and build references from the
> 	newly built statements.
> 	* ipa-ref.c (ipa_remove_stmt_references): New function.
> 	* ipa-ref.h (ipa_remove_stmt_references): Declare.
> 
> testsuite/
> 	* gcc.dg/ipa/pr57294.c: New test.
> 
> +
> +void
> +ipa_remove_stmt_references (symtab_node referring_node, gimple stmt)

Missing comment.
Otherwise the patch is OK.
It would be nice to modify ipa_record_stmt_references to allow also verification that all
references are recorded. 

Honza

Patch

Index: src/gcc/cgraph.h
===================================================================
--- src.orig/gcc/cgraph.h
+++ src/gcc/cgraph.h
@@ -705,6 +705,7 @@  unsigned int rebuild_cgraph_edges (void)
 void cgraph_rebuild_references (void);
 int compute_call_stmt_bb_frequency (tree, basic_block bb);
 void record_references_in_initializer (tree, bool);
+void ipa_record_stmt_references (struct cgraph_node *, gimple);
 
 /* In ipa.c  */
 bool symtab_remove_unreachable_nodes (bool, FILE *);
Index: src/gcc/cgraphbuild.c
===================================================================
--- src.orig/gcc/cgraphbuild.c
+++ src/gcc/cgraphbuild.c
@@ -288,6 +288,14 @@  mark_store (gimple stmt, tree t, void *d
   return false;
 }
 
+/* Record all references from NODE that are taken in statement STMT.  */
+void
+ipa_record_stmt_references (struct cgraph_node *node, gimple stmt)
+{
+  walk_stmt_load_store_addr_ops (stmt, node, mark_load, mark_store,
+				 mark_address);
+}
+
 /* Create cgraph edges for function calls.
    Also look for functions and variables having addresses taken.  */
 
@@ -323,8 +331,7 @@  build_cgraph_edges (void)
 					     gimple_call_flags (stmt),
 					     bb->count, freq);
 	    }
-	  walk_stmt_load_store_addr_ops (stmt, node, mark_load,
-					 mark_store, mark_address);
+	  ipa_record_stmt_references (node, stmt);
 	  if (gimple_code (stmt) == GIMPLE_OMP_PARALLEL
 	      && gimple_omp_parallel_child_fn (stmt))
 	    {
@@ -348,8 +355,7 @@  build_cgraph_edges (void)
 	    }
 	}
       for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-	walk_stmt_load_store_addr_ops (gsi_stmt (gsi), node,
-				       mark_load, mark_store, mark_address);
+	ipa_record_stmt_references (node, gsi_stmt (gsi));
    }
 
   /* Look for initializers of constant variables and private statics.  */
@@ -437,13 +443,10 @@  rebuild_cgraph_edges (void)
 					     gimple_call_flags (stmt),
 					     bb->count, freq);
 	    }
-	  walk_stmt_load_store_addr_ops (stmt, node, mark_load,
-					 mark_store, mark_address);
-
+	  ipa_record_stmt_references (node, stmt);
 	}
       for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-	walk_stmt_load_store_addr_ops (gsi_stmt (gsi), node,
-				       mark_load, mark_store, mark_address);
+	ipa_record_stmt_references (node, gsi_stmt (gsi));
     }
   record_eh_tables (node, cfun);
   gcc_assert (!node->global.inlined_to);
@@ -468,16 +471,9 @@  cgraph_rebuild_references (void)
   FOR_EACH_BB (bb)
     {
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-	{
-	  gimple stmt = gsi_stmt (gsi);
-
-	  walk_stmt_load_store_addr_ops (stmt, node, mark_load,
-					 mark_store, mark_address);
-
-	}
+	ipa_record_stmt_references (node, gsi_stmt (gsi));
       for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-	walk_stmt_load_store_addr_ops (gsi_stmt (gsi), node,
-				       mark_load, mark_store, mark_address);
+	ipa_record_stmt_references (node, gsi_stmt (gsi));
     }
   record_eh_tables (node, cfun);
 }
Index: src/gcc/ipa-prop.c
===================================================================
--- src.orig/gcc/ipa-prop.c
+++ src/gcc/ipa-prop.c
@@ -3216,18 +3216,22 @@  void
 ipa_modify_call_arguments (struct cgraph_edge *cs, gimple stmt,
 			   ipa_parm_adjustment_vec adjustments)
 {
+  struct cgraph_node *current_node = cgraph_get_node (current_function_decl);
   vec<tree> vargs;
   vec<tree, va_gc> **debug_args = NULL;
   gimple new_stmt;
-  gimple_stmt_iterator gsi;
+  gimple_stmt_iterator gsi, prev_gsi;
   tree callee_decl;
   int i, len;
 
   len = adjustments.length ();
   vargs.create (len);
   callee_decl = !cs ? gimple_call_fndecl (stmt) : cs->callee->symbol.decl;
+  ipa_remove_stmt_references ((symtab_node) current_node, stmt);
 
   gsi = gsi_for_stmt (stmt);
+  prev_gsi = gsi;
+  gsi_prev (&prev_gsi);
   for (i = 0; i < len; i++)
     {
       struct ipa_parm_adjustment *adj;
@@ -3422,6 +3426,14 @@  ipa_modify_call_arguments (struct cgraph
   gsi_replace (&gsi, new_stmt, true);
   if (cs)
     cgraph_set_call_stmt (cs, new_stmt);
+  do
+    {
+      ipa_record_stmt_references (current_node, gsi_stmt (gsi));
+      gsi_prev (&gsi);
+    }
+  while ((gsi_end_p (prev_gsi) && !gsi_end_p (gsi))
+	 || (!gsi_end_p (prev_gsi) && gsi_stmt (gsi) == gsi_stmt (prev_gsi)));
+
   update_ssa (TODO_update_ssa);
   free_dominance_info (CDI_DOMINATORS);
 }
Index: src/gcc/ipa-ref.c
===================================================================
--- src.orig/gcc/ipa-ref.c
+++ src/gcc/ipa-ref.c
@@ -215,3 +215,14 @@  ipa_find_reference (symtab_node referrin
       return r;
   return NULL;
 }
+
+void
+ipa_remove_stmt_references (symtab_node referring_node, gimple stmt)
+{
+  struct ipa_ref *r = NULL;
+  int i;
+
+  FOR_EACH_VEC_SAFE_ELT (referring_node->symbol.ref_list.references, i, r)
+    if (r->stmt == stmt)
+      ipa_remove_reference (r);
+}
Index: src/gcc/ipa-ref.h
===================================================================
--- src.orig/gcc/ipa-ref.h
+++ src/gcc/ipa-ref.h
@@ -72,3 +72,4 @@  void ipa_clone_referring (symtab_node, s
 bool ipa_ref_cannot_lead_to_return (struct ipa_ref *);
 bool ipa_ref_has_aliases_p (struct ipa_ref_list *);
 struct ipa_ref * ipa_find_reference (symtab_node, symtab_node, gimple);
+void ipa_remove_stmt_references (symtab_node, gimple);
Index: src/gcc/testsuite/gcc.dg/ipa/pr57294.c
===================================================================
--- /dev/null
+++ src/gcc/testsuite/gcc.dg/ipa/pr57294.c
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+void baz (void);
+int func ();
+
+static void
+bar (int a, int foo (void))
+{
+  baz ();
+  foo ();
+}
+
+void
+baz (void)
+{
+  bar (0, func);
+}