This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix target_reinit (PR target/47564)


Hi!

This patch fixes a target attribute handling bug.  set_cfun when changing
from a function with one target attribute (or missing) to one with a
different target attribute calls target_reinit, which is unfortunately quite
inefficient, doesn't cache anything and, what's worse, initializes rtl
generation and cleans it up afterwards for init_expmed, init_expr_target,
init_set_costs and ira_init purposes.  When set_cfun with such target_reinit
side-effect is called after prepare_function_start call (called very early
in tree_rest_of_compilation), it clears *crtl and thus e.g. during expansion
when allocating pseudos they don't start at LAST_VIRTUAL_REGISTER + 1, but
at 0.

For 4.7 it would be nice if we could cache results of as many of these init_*
functions as possible, at least for those that are used before rtl
generation (e.g. init_set_costs), and for init_* calls whose results are
only ever used in between expand_gimple_cfg and following
free_after_compilation we could just call them at the start of
expand_gimple_cfg if target attribute changed since last expansion (e.g.
ira_init would be hard to save/restore).

For 4.6 that would be too risky, so this patch instead just saves/restores
what free_after_compilation clobbers.  Initially I thought I'd need to
register those with GC, but looking at the source I think ggc_collect ()
can't be called during target_reinit (it would break quite a few passes that
do set_cfun at various spots temporarily if set_cfun could collect).

The cgraph verification changes are just to make the verification cheaper
(target_reinit is very costly and if we can avoid it easily...).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2011-02-02  Jakub Jelinek  <jakub@redhat.com>

	PR target/47564
	* toplev.c (target_reinit): Save and restore *crtl and regno_reg_rtx
	around backend_init_target and lang_dependent_init_target calls.
	* cgraphunit.c (cgraph_debug_gimple_stmt): New function.
	(verify_cgraph_node): Don't call set_cfun here.  Use
	cgraph_debug_gimple_stmt instead of debug_gimple_stmt.
	Set error_found for incorrectly represented calls to thunks.

	* gcc.target/i386/pr47564.c: New test.

--- gcc/toplev.c.jj	2011-02-01 16:23:54.000000000 +0100
+++ gcc/toplev.c	2011-02-02 10:40:15.000000000 +0100
@@ -1780,11 +1780,33 @@ lang_dependent_init (const char *name)
 void
 target_reinit (void)
 {
+  struct rtl_data saved_x_rtl;
+  rtx *saved_regno_reg_rtx;
+
+  /* Save *crtl and regno_reg_rtx around the reinitialization
+     to allow target_reinit being called even after prepare_function_start.  */
+  saved_regno_reg_rtx = regno_reg_rtx;
+  if (saved_regno_reg_rtx)
+    {  
+      saved_x_rtl = *crtl;
+      memset (crtl, '\0', sizeof (*crtl));
+      regno_reg_rtx = NULL;
+    }
+
   /* Reinitialize RTL backend.  */
   backend_init_target ();
 
   /* Reinitialize lang-dependent parts.  */
   lang_dependent_init_target ();
+
+  /* And restore it at the end, as free_after_compilation from
+     expand_dummy_function_end clears it.  */
+  if (saved_regno_reg_rtx)
+    {
+      *crtl = saved_x_rtl;
+      regno_reg_rtx = saved_regno_reg_rtx;
+      saved_regno_reg_rtx = NULL;
+    }
 }
 
 void
--- gcc/cgraphunit.c.jj	2011-01-31 14:11:30.000000000 +0100
+++ gcc/cgraphunit.c	2011-02-01 15:10:50.000000000 +0100
@@ -440,13 +440,22 @@ verify_edge_count_and_frequency (struct 
   return error_found;
 }
 
+/* Switch to THIS_CFUN if needed and print STMT to stderr.  */
+static void
+cgraph_debug_gimple_stmt (struct function *this_cfun, gimple stmt)
+{
+  /* debug_gimple_stmt needs correct cfun */
+  if (cfun != this_cfun)
+    set_cfun (this_cfun);
+  debug_gimple_stmt (stmt);
+}
+
 /* Verify cgraph nodes of given cgraph node.  */
 DEBUG_FUNCTION void
 verify_cgraph_node (struct cgraph_node *node)
 {
   struct cgraph_edge *e;
   struct function *this_cfun = DECL_STRUCT_FUNCTION (node->decl);
-  struct function *saved_cfun = cfun;
   basic_block this_block;
   gimple_stmt_iterator gsi;
   bool error_found = false;
@@ -455,8 +464,6 @@ 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)
       {
@@ -499,7 +506,7 @@ verify_cgraph_node (struct cgraph_node *
 	  error ("An indirect edge from %s is not marked as indirect or has "
 		 "associated indirect_info, the corresponding statement is: ",
 		 identifier_to_locale (cgraph_node_name (e->caller)));
-	  debug_gimple_stmt (e->call_stmt);
+	  cgraph_debug_gimple_stmt (this_cfun, e->call_stmt);
 	  error_found = true;
 	}
     }
@@ -642,7 +649,7 @@ verify_cgraph_node (struct cgraph_node *
 			if (e->aux)
 			  {
 			    error ("shared call_stmt:");
-			    debug_gimple_stmt (stmt);
+			    cgraph_debug_gimple_stmt (this_cfun, stmt);
 			    error_found = true;
 			  }
 			if (!e->indirect_unknown_callee)
@@ -676,7 +683,8 @@ verify_cgraph_node (struct cgraph_node *
 			      {
 				error ("a call to thunk improperly represented "
 				       "in the call graph:");
-				debug_gimple_stmt (stmt);
+				cgraph_debug_gimple_stmt (this_cfun, stmt);
+				error_found = true;
 			      }
 			  }
 			else if (decl)
@@ -685,14 +693,14 @@ verify_cgraph_node (struct cgraph_node *
 				   "corresponding to a call_stmt with "
 				   "a known declaration:");
 			    error_found = true;
-			    debug_gimple_stmt (e->call_stmt);
+			    cgraph_debug_gimple_stmt (this_cfun, e->call_stmt);
 			  }
 			e->aux = (void *)1;
 		      }
 		    else if (decl)
 		      {
 			error ("missing callgraph edge for call stmt:");
-			debug_gimple_stmt (stmt);
+			cgraph_debug_gimple_stmt (this_cfun, stmt);
 			error_found = true;
 		      }
 		  }
@@ -710,7 +718,7 @@ verify_cgraph_node (struct cgraph_node *
 	      error ("edge %s->%s has no corresponding call_stmt",
 		     identifier_to_locale (cgraph_node_name (e->caller)),
 		     identifier_to_locale (cgraph_node_name (e->callee)));
-	      debug_gimple_stmt (e->call_stmt);
+	      cgraph_debug_gimple_stmt (this_cfun, e->call_stmt);
 	      error_found = true;
 	    }
 	  e->aux = 0;
@@ -721,7 +729,7 @@ verify_cgraph_node (struct cgraph_node *
 	    {
 	      error ("an indirect edge from %s has no corresponding call_stmt",
 		     identifier_to_locale (cgraph_node_name (e->caller)));
-	      debug_gimple_stmt (e->call_stmt);
+	      cgraph_debug_gimple_stmt (this_cfun, e->call_stmt);
 	      error_found = true;
 	    }
 	  e->aux = 0;
@@ -732,7 +740,6 @@ 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.target/i386/pr47564.c.jj	2011-02-02 10:24:34.000000000 +0100
+++ gcc/testsuite/gcc.target/i386/pr47564.c	2011-02-02 10:27:22.000000000 +0100
@@ -0,0 +1,42 @@
+/* PR target/47564 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -msse2" } */
+
+static inline unsigned long long
+foo (const unsigned char *p)
+{
+  return 1;
+}
+
+__attribute__ ((__target__ ("sse4"))) void
+bar (unsigned long long *x, const void *b, long long m)
+{
+  const unsigned char *p = (const unsigned char *) b;
+  const unsigned char *e = p + m;
+  unsigned int l = *x;
+  unsigned long long n = l;
+
+  if ((e - p) >= 8192)
+    {
+      while ((e - p) >= 128)
+	{
+	  n = __builtin_ia32_crc32di (n, foo (p));
+	  n = __builtin_ia32_crc32di (n, foo (p));
+	  n = __builtin_ia32_crc32di (n, foo (p));
+	  n = __builtin_ia32_crc32di (n, foo (p));
+	  n = __builtin_ia32_crc32di (n, foo (p));
+	  n = __builtin_ia32_crc32di (n, foo (p));
+	  n = __builtin_ia32_crc32di (n, foo (p));
+	  n = __builtin_ia32_crc32di (n, foo (p));
+	  n = __builtin_ia32_crc32di (n, foo (p));
+	}
+    }
+
+  while ((e - p) >= 16)
+    {
+      n = __builtin_ia32_crc32di (n, foo (p));
+      n = __builtin_ia32_crc32di (n, foo (p));
+    }
+  l = n;
+  *x = l;
+}

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]