Patchwork [PR,55579] Make SRA keep candidated with only debug replacements

login
register
mail settings
Submitter Martin Jambor
Date Jan. 4, 2013, 1 p.m.
Message ID <20130104130039.GB5183@virgil.suse>
Download mbox | patch
Permalink /patch/209452/
State New
Headers show

Comments

Martin Jambor - Jan. 4, 2013, 1 p.m.
Hi,

the patch below fixes PR 55579 by not disqualifying a candidate
variable when it does not have any real replacements but has
ones for debug statements only.

analyze_access_subtree has to return the same value regardless of
MAY_HAVE_DEBUG_STMTS or we get debug comparison errors.  I also have a
patch that simply avoids disqualifying candidates with no replacements
which means we keep some information about it so that SRA can remove a
few more un-needed reads but I suppose that is stage1 material.

OK for trunk?

Thanks,

Martin


2013-01-03  Martin Jambor  <mjambor@suse.cz>

	PR debug/55579
	* tree-sra.c (analyze_access_subtree): Return true also after
	potentially creating a debug-only replacement.

testsuite/
	* gcc.dg/tree-ssa/pr55579.c: New test.
Jakub Jelinek - Jan. 8, 2013, 11:51 a.m.
On Fri, Jan 04, 2013 at 02:00:39PM +0100, Martin Jambor wrote:
> 2013-01-03  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR debug/55579
> 	* tree-sra.c (analyze_access_subtree): Return true also after
> 	potentially creating a debug-only replacement.
> 
> testsuite/
> 	* gcc.dg/tree-ssa/pr55579.c: New test.

Looks good, thanks.

	Jakub

Patch

Index: src/gcc/testsuite/gcc.dg/tree-ssa/pr55579.c
===================================================================
--- /dev/null
+++ src/gcc/testsuite/gcc.dg/tree-ssa/pr55579.c
@@ -0,0 +1,14 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -g -fdump-tree-esra" } */
+
+struct S { int a; char b; char c; short d; };
+
+int
+foo (int x)
+{
+  struct S s = { x + 1, x + 2, x + 3, x + 4 };
+  char *p = &s.c;
+  return x;
+}
+
+/* { dg-final { scan-tree-dump "Created a debug-only replacement for s" "esra" } } */
Index: src/gcc/tree-sra.c
===================================================================
--- src.orig/gcc/tree-sra.c
+++ src/gcc/tree-sra.c
@@ -2197,20 +2197,25 @@  analyze_access_subtree (struct access *r
     }
   else
     {
-      if (MAY_HAVE_DEBUG_STMTS && allow_replacements
+      if (allow_replacements
 	  && scalar && !root->first_child
 	  && (root->grp_scalar_write || root->grp_assignment_write))
 	{
 	  gcc_checking_assert (!root->grp_scalar_read
 			       && !root->grp_assignment_read);
-	  root->grp_to_be_debug_replaced = 1;
-	  if (dump_file && (dump_flags & TDF_DETAILS))
+	  sth_created = true;
+	  if (MAY_HAVE_DEBUG_STMTS)
 	    {
-	      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->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");
+		}
 	    }
 	}
 
@@ -2220,17 +2225,11 @@  analyze_access_subtree (struct access *r
 	root->grp_total_scalarization = 0;
     }
 
-  if (sth_created
-      && (!hole || root->grp_total_scalarization))
-    {
-      root->grp_covered = 1;
-      return true;
-    }
-  if (root->grp_write || TREE_CODE (root->base) == PARM_DECL)
+  if (!hole || root->grp_total_scalarization)
+    root->grp_covered = 1;
+  else if (root->grp_write || TREE_CODE (root->base) == PARM_DECL)
     root->grp_unscalarized_data = 1; /* not covered and written to */
-  if (sth_created)
-    return true;
-  return false;
+  return sth_created;
 }
 
 /* Analyze all access trees linked by next_grp by the means of