Fix regression on PR46590 (slow compile with -O0)

Michael Matz matz@suse.de
Mon Jan 23 17:01:00 GMT 2012


Hi,

On Sat, 21 Jan 2012, Eric Botcazou wrote:

> > Trivially fixing the thinko (iterating over (work bit-and 
> > old_conflict) in the first inner loop) would fix the testcase but in 
> > general create too few conflicts, i.e. generate wrong code.  I need 
> > some time to think about this again.
> 
> OK, thanks in advance.

Sigh.  I can't come up with a way to generally speed up the conflict 
generation without sometimes adding artificial conflicts.  I.e. I'll have 
to revert r183305.  I can still fix the specific situation of PR46590 by 
making sure clobbers are added for all aggregates, not only the (at 
gimplification time) address takens.

So, this is currently in regstrapping, x86_64-linux, all langs+Ada.  Okay 
for trunk (well, the revert is obvious, but the gimplify.c hunk)?


Ciao,
Michael.

	PR tree-optimization/46590
	* cfgexpand.c: Revert last change (r183305).
	* gimplify.c (gimplify_bind_expr): Add clobbers for all non-gimple
	regs.

Index: gimplify.c
===================================================================
--- gimplify.c	(revision 183303)
+++ gimplify.c	(working copy)
@@ -1226,12 +1226,11 @@ gimplify_bind_expr (tree *expr_p, gimple
       if (TREE_CODE (t) == VAR_DECL
 	  && !is_global_var (t)
 	  && DECL_CONTEXT (t) == current_function_decl
-	  && !DECL_HARD_REGISTER (t)
-	  && !TREE_THIS_VOLATILE (t)
 	  && !DECL_HAS_VALUE_EXPR_P (t)
 	  /* Only care for variables that have to be in memory.  Others
 	     will be rewritten into SSA names, hence moved to the top-level.  */
-	  && needs_to_live_in_memory (t))
+	  && !is_gimple_reg (t))
+
 	{
 	  tree clobber = build_constructor (TREE_TYPE (t), NULL);
 	  TREE_THIS_VOLATILE (clobber) = 1;
Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 183305)
+++ cfgexpand.c	(working copy)
@@ -441,12 +441,11 @@ visit_conflict (gimple stmt ATTRIBUTE_UN
 
 /* Helper routine for add_scope_conflicts, calculating the active partitions
    at the end of BB, leaving the result in WORK.  We're called to generate
-   conflicts when OLD_CONFLICTS is non-null, otherwise we're just tracking
-   liveness.  If we generate conflicts then OLD_CONFLICTS stores the bits
-   for which we generated conflicts already.  */
+   conflicts when FOR_CONFLICT is true, otherwise we're just tracking
+   liveness.  */
 
 static void
-add_scope_conflicts_1 (basic_block bb, bitmap work, bitmap old_conflicts)
+add_scope_conflicts_1 (basic_block bb, bitmap work, bool for_conflict)
 {
   edge e;
   edge_iterator ei;
@@ -483,7 +482,7 @@ add_scope_conflicts_1 (basic_block bb, b
 	}
       else if (!is_gimple_debug (stmt))
 	{
-	  if (old_conflicts
+	  if (for_conflict
 	      && visit == visit_op)
 	    {
 	      /* If this is the first real instruction in this BB we need
@@ -491,27 +490,16 @@ add_scope_conflicts_1 (basic_block bb, b
 		 Unlike classical liveness for named objects we can't
 		 rely on seeing a def/use of the names we're interested in.
 		 There might merely be indirect loads/stores.  We'd not add any
-		 conflicts for such partitions.  We know that we generated
-		 conflicts between all partitions in old_conflicts already,
-		 so we need to generate only the new ones, avoiding to
-		 repeatedly pay the O(N^2) cost for each basic block.  */
+		 conflicts for such partitions.  */
 	      bitmap_iterator bi;
 	      unsigned i;
-
-	      EXECUTE_IF_AND_COMPL_IN_BITMAP (work, old_conflicts, 0, i, bi)
+	      EXECUTE_IF_SET_IN_BITMAP (work, 0, i, bi)
 		{
 		  unsigned j;
 		  bitmap_iterator bj;
-		  /* First the conflicts between new and old_conflicts.  */
-		  EXECUTE_IF_SET_IN_BITMAP (old_conflicts, 0, j, bj)
-		    add_stack_var_conflict (i, j);
-		  /* Then the conflicts between only the new members.  */
-		  EXECUTE_IF_AND_COMPL_IN_BITMAP (work, old_conflicts, i + 1,
-						  j, bj)
+		  EXECUTE_IF_SET_IN_BITMAP (work, i + 1, j, bj)
 		    add_stack_var_conflict (i, j);
 		}
-	      /* And remember for the next basic block.  */
-	      bitmap_ior_into (old_conflicts, work);
 	      visit = visit_conflict;
 	    }
 	  walk_stmt_load_store_addr_ops (stmt, work, visit, visit, visit);
@@ -528,7 +516,6 @@ add_scope_conflicts (void)
   basic_block bb;
   bool changed;
   bitmap work = BITMAP_ALLOC (NULL);
-  bitmap old_conflicts;
 
   /* We approximate the live range of a stack variable by taking the first
      mention of its name as starting point(s), and by the end-of-scope
@@ -550,18 +537,15 @@ add_scope_conflicts (void)
       FOR_EACH_BB (bb)
 	{
 	  bitmap active = (bitmap)bb->aux;
-	  add_scope_conflicts_1 (bb, work, NULL);
+	  add_scope_conflicts_1 (bb, work, false);
 	  if (bitmap_ior_into (active, work))
 	    changed = true;
 	}
     }
 
-  old_conflicts = BITMAP_ALLOC (NULL);
-
   FOR_EACH_BB (bb)
-    add_scope_conflicts_1 (bb, work, old_conflicts);
+    add_scope_conflicts_1 (bb, work, true);
 
-  BITMAP_FREE (old_conflicts);
   BITMAP_FREE (work);
   FOR_ALL_BB (bb)
     BITMAP_FREE (bb->aux);



More information about the Gcc-patches mailing list