Re: [PATCH, PR 43905] Make IPA-SRA clone functions it modifies

[Martin Jambor <mjambor at suse dot cz>]

Hi,

On Thu, Jun 10, 2010 at 11:40:06PM +0200, Martin Jambor wrote:
> Hi,
> 
> the patch below is basically a different fix to PR 43141 which fixes
> the still-opened PR 43905 and a number of issues that that first
> attempt caused (e.g. wrong sharing of parameter chain list and some
> debug info problems).  After trying quite hard to move the body of a
> function from the old declaration to a new one and all necessary
> fixing up so that nothing breaks, I gave up and resorted to simple
> cloning.
> 
> Fortunately the pass management works in a way that setting the cfun
> and current_function_decl to the new function is enough to make the
> subsequent passes work on the correct function.  It is a bit of a
> hack, but I can't think of any substantial improvement.
> 

The previous patch did not test whether the current function was
versionable which lead to segfaults when I supplied a noclone
attribute to some simple testcases.  This is an updated patch that
does perform the necessary check (which unfortunately requires an
extra include), otherwise it is exactly the same.

Again, bootstrapped and tested on x86_64-linux without any issues.  OK
for trunk and later for 4.5 as well?  

Honza, it seems everybody is waiting for your assessment :-)

Thanks,

Martin


2010-06-14  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/43905
	* tree-sra.c: Include tree-inline.h.
	(create_abstract_origin): Removed.
	(modify_function): Version the call graph node instead of creating
	abstract origins and dealing with same_body aliases.
	* tree-sra.c (ipa_sra_preliminary_function_checks): Check whether the
	function is versionable.
	* Makefile.in (tree-sra.o): Add TREE_INLINE_H to dependencies.

	* testsuite/g++.dg/torture/pr43905.C

Index: mine/gcc/tree-sra.c
===================================================================
--- mine.orig/gcc/tree-sra.c
+++ mine/gcc/tree-sra.c
@@ -89,6 +89,7 @@ along with GCC; see the file COPYING3.
 #include "target.h"
 #include "flags.h"
 #include "dbgcnt.h"
+#include "tree-inline.h"
 
 /* Enumeration of all aggregate reductions we can do.  */
 enum sra_mode { SRA_MODE_EARLY_IPA,   /* early call regularization */
@@ -4224,43 +4225,38 @@ convert_callers (struct cgraph_node *nod
   return;
 }
 
-/* Create an abstract origin declaration for OLD_DECL and make it an abstract
-   origin of the provided decl so that there are preserved parameters for debug
-   information.  */
-
-static void
-create_abstract_origin (tree old_decl)
-{
-  if (!DECL_ABSTRACT_ORIGIN (old_decl))
-    {
-      tree new_decl = copy_node (old_decl);
-
-      DECL_ABSTRACT (new_decl) = 1;
-      SET_DECL_ASSEMBLER_NAME (new_decl, NULL_TREE);
-      SET_DECL_RTL (new_decl, NULL);
-      DECL_STRUCT_FUNCTION (new_decl) = NULL;
-      DECL_ARTIFICIAL (old_decl) = 1;
-      DECL_ABSTRACT_ORIGIN (old_decl) = new_decl;
-    }
-}
-
 /* Perform all the modification required in IPA-SRA for NODE to have parameters
    as given in ADJUSTMENTS.  */
 
 static void
 modify_function (struct cgraph_node *node, ipa_parm_adjustment_vec adjustments)
 {
-  struct cgraph_node *alias;
-  for (alias = node->same_body; alias; alias = alias->next)
-    ipa_modify_formal_parameters (alias->decl, adjustments, "ISRA");
-  /* current_function_decl must be handled last, after same_body aliases,
-     as following functions will use what it computed.  */
-  create_abstract_origin (current_function_decl);
+  struct cgraph_node *new_node;
+  struct cgraph_edge *cs;
+  VEC (cgraph_edge_p, heap) * redirect_callers;
+  int node_callers;
+
+  node_callers = 0;
+  for (cs = node->callers; cs != NULL; cs = cs->next_caller)
+    node_callers++;
+  redirect_callers = VEC_alloc (cgraph_edge_p, heap, node_callers);
+  for (cs = node->callers; cs != NULL; cs = cs->next_caller)
+    VEC_quick_push (cgraph_edge_p, redirect_callers, cs);
+
+  rebuild_cgraph_edges ();
+  pop_cfun ();
+  current_function_decl = NULL_TREE;
+
+  new_node = cgraph_function_versioning (node, redirect_callers, NULL, NULL,
+					 NULL, NULL, "isra");
+  current_function_decl = new_node->decl;
+  push_cfun (DECL_STRUCT_FUNCTION (new_node->decl));
+
   ipa_modify_formal_parameters (current_function_decl, adjustments, "ISRA");
   ipa_sra_modify_function_body (adjustments);
   sra_ipa_reset_debug_stmts (adjustments);
-  convert_callers (node, adjustments);
-  cgraph_make_node_local (node);
+  convert_callers (new_node, adjustments);
+  cgraph_make_node_local (new_node);
   return;
 }
 
@@ -4275,6 +4271,13 @@ ipa_sra_preliminary_function_checks (str
     {
       if (dump_file)
 	fprintf (dump_file, "Function not local to this compilation unit.\n");
+      return false;
+    }
+
+  if (!tree_versionable_function_p (node->decl))
+    {
+      if (dump_file)
+	fprintf (dump_file, "Function not local to this compilation unit.\n");
       return false;
     }
 
Index: mine/gcc/testsuite/g++.dg/torture/pr43905.C
===================================================================
--- /dev/null
+++ mine/gcc/testsuite/g++.dg/torture/pr43905.C
@@ -0,0 +1,13 @@
+extern void sf ( __const char *);
+struct Matrix{
+  int operator[](int n){
+    sf ( __PRETTY_FUNCTION__);
+  }
+  int operator[](int n)const{
+    sf ( __PRETTY_FUNCTION__);
+  }
+};
+void calcmy(Matrix const &b, Matrix &c, int k){
+  b[k];
+  c[k];
+}
Index: mine/gcc/Makefile.in
===================================================================
--- mine.orig/gcc/Makefile.in
+++ mine/gcc/Makefile.in
@@ -3118,7 +3118,8 @@ tree-ssa-ccp.o : tree-ssa-ccp.c $(TREE_F
 tree-sra.o : tree-sra.c $(CONFIG_H) $(SYSTEM_H) coretypes.h alloc-pool.h \
    $(TM_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) $(IPA_PROP_H) \
    $(DIAGNOSTIC_H) statistics.h $(TREE_DUMP_H) $(TIMEVAR_H) $(PARAMS_H) \
-   $(TARGET_H) $(FLAGS_H) $(EXPR_H) tree-pretty-print.h $(DBGCNT_H)
+   $(TARGET_H) $(FLAGS_H) $(EXPR_H) tree-pretty-print.h $(DBGCNT_H) \
+   $(TREE_INLINE_H)
 tree-switch-conversion.o : tree-switch-conversion.c $(CONFIG_H) $(SYSTEM_H) \
     $(TREE_H) $(TM_P_H) $(TREE_FLOW_H) $(DIAGNOSTIC_H) $(TREE_INLINE_H) \
     $(TIMEVAR_H) $(TM_H) coretypes.h $(TREE_DUMP_H) $(GIMPLE_H) \