Patchwork [PR,56294] Create all replacements upfront

login
register
mail settings
Submitter Martin Jambor
Date Feb. 27, 2013, 2:52 p.m.
Message ID <20130227145220.GB17127@virgil.suse>
Download mbox | patch
Permalink /patch/223622/
State New
Headers show

Comments

Martin Jambor - Feb. 27, 2013, 2:52 p.m.
Hi,

this patch makes SRA create all scheduled replacements declarations
before starting the modification pass over the function.  Because we
now create more replacements when generating debug info than when we
do not, this is the only way we can be sure the declarations (even
those that eventually happen in non-debug statements) are created in
the same order in -g and -g0, their UIDs have the same order and thus
do not cause inadvertent code generation differences later.  And we
also call create_tmp_var the same number of times which was causing
one of the PR 56294 problems in the first place.  There is still at
least one outstanding issue, I'll update the bug shortly with what I
know so far about it.

I chucked out some dumping from analyze_access_subtree because newly
called create_access_replacement dumps basically the same thing.

Bootstrapped and tested on x86_64-linux, fixes an -fdebug-compare
regression.  OK for trunk?

Thanks,

Martin


2013-02-25  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/56294
	* tree-sra.c (analyze_access_subtree): Create replacement declarations.
	Adjust dumping.
	(get_access_replacement): Do not call create_access_replacement.
	Assert a replacement exists.
	(get_repl_default_def_ssa_name): Create the replacement declaration
	itself.

testsuite/
	* g++.dg/debug/pr56294.C: New test.
Jakub Jelinek - Feb. 27, 2013, 3 p.m.
On Wed, Feb 27, 2013 at 03:52:20PM +0100, Martin Jambor wrote:
> Bootstrapped and tested on x86_64-linux, fixes an -fdebug-compare
> regression.  OK for trunk?

Ok, thanks.

> 2013-02-25  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR tree-optimization/56294
> 	* tree-sra.c (analyze_access_subtree): Create replacement declarations.
> 	Adjust dumping.
> 	(get_access_replacement): Do not call create_access_replacement.
> 	Assert a replacement exists.
> 	(get_repl_default_def_ssa_name): Create the replacement declaration
> 	itself.
> 
> testsuite/
> 	* g++.dg/debug/pr56294.C: New test.

	Jakub

Patch

Index: src/gcc/tree-sra.c
===================================================================
--- src.orig/gcc/tree-sra.c
+++ src/gcc/tree-sra.c
@@ -2000,8 +2000,7 @@  create_access_replacement (struct access
 static inline tree
 get_access_replacement (struct access *access)
 {
-  if (!access->replacement_decl)
-    access->replacement_decl = create_access_replacement (access);
+  gcc_checking_assert (access->replacement_decl);
   return access->replacement_decl;
 }
 
@@ -2157,7 +2156,6 @@  analyze_access_subtree (struct access *r
 	  || ((root->grp_scalar_read || root->grp_assignment_read)
 	      && (root->grp_scalar_write || root->grp_assignment_write))))
     {
-      bool new_integer_type;
       /* Always create access replacements that cover the whole access.
          For integral types this means the precision has to match.
 	 Avoid assumptions based on the integral type kind, too.  */
@@ -2176,22 +2174,19 @@  analyze_access_subtree (struct access *r
 	  root->expr = build_ref_for_offset (UNKNOWN_LOCATION,
 					     root->base, root->offset,
 					     root->type, NULL, false);
-	  new_integer_type = true;
-	}
-      else
-	new_integer_type = false;
 
-      if (dump_file && (dump_flags & TDF_DETAILS))
-	{
-	  fprintf (dump_file, "Marking ");
-	  print_generic_expr (dump_file, root->base, 0);
-	  fprintf (dump_file, " offset: %u, size: %u ",
-		   (unsigned) root->offset, (unsigned) root->size);
-	  fprintf (dump_file, " to be replaced%s.\n",
-		   new_integer_type ? " with an integer": "");
+	  if (dump_file && (dump_flags & TDF_DETAILS))
+	    {
+	      fprintf (dump_file, "Changing the type of a replacement for ");
+	      print_generic_expr (dump_file, root->base, 0);
+	      fprintf (dump_file, " offset: %u, size: %u ",
+		       (unsigned) root->offset, (unsigned) root->size);
+	      fprintf (dump_file, " to an integer.\n");
+	    }
 	}
 
       root->grp_to_be_replaced = 1;
+      root->replacement_decl = create_access_replacement (root);
       sth_created = true;
       hole = false;
     }
@@ -2209,15 +2204,7 @@  analyze_access_subtree (struct access *r
 	  if (MAY_HAVE_DEBUG_STMTS)
 	    {
 	      root->grp_to_be_debug_replaced = 1;
-	      if (dump_file && (dump_flags & TDF_DETAILS))
-		{
-		  fprintf (dump_file, "Marking ");
-		  print_generic_expr (dump_file, root->base, 0);
-		  fprintf (dump_file, " offset: %u, size: %u ",
-			   (unsigned) root->offset, (unsigned) root->size);
-		  fprintf (dump_file, " to be replaced with debug "
-			   "statements.\n");
-		}
+	      root->replacement_decl = create_access_replacement (root);
 	    }
 	}
 
@@ -2973,7 +2960,11 @@  sra_modify_constructor_assign (gimple *s
 static tree
 get_repl_default_def_ssa_name (struct access *racc)
 {
-  return get_or_create_ssa_default_def (cfun, get_access_replacement (racc));
+  gcc_checking_assert (!racc->grp_to_be_replaced &&
+		       !racc->grp_to_be_debug_replaced);
+  if (!racc->replacement_decl)
+    racc->replacement_decl = create_access_replacement (racc);
+  return get_or_create_ssa_default_def (cfun, racc->replacement_decl);
 }
 
 /* Return true if REF has a COMPONENT_REF with a bit-field field declaration
Index: src/gcc/testsuite/g++.dg/debug/pr56294.C
===================================================================
--- /dev/null
+++ src/gcc/testsuite/g++.dg/debug/pr56294.C
@@ -0,0 +1,30 @@ 
+// { dg-do compile }
+// { dg-options "-fno-ipa-sra -fcompare-debug" }
+
+struct comp_cost { int cost; unsigned complexity; };
+struct cost_pair { struct iv_cand *cand; };
+struct iv_use { unsigned n_map_members; cost_pair *cost_map; };
+struct iv_cand { unsigned id; };
+
+unsigned gu;
+
+void
+bar (comp_cost, comp_cost)
+{
+}
+
+void
+foo (iv_use *use, iv_cand *cand)
+{
+  unsigned i, s = cand->id & (use->n_map_members - 1);
+  for (i = 0; i < s; i++)
+    if (use->cost_map[i].cand)
+      goto found;
+found:
+  use->cost_map[i].cand = cand;
+  comp_cost elim_cost, express_cost, bound_cost;
+  bar (elim_cost, express_cost);
+  gu = express_cost.complexity;
+}
+
+