[PATCH, PR 52693] Do not construct memory accesses to unscalarizable regions

[PATCH, PR 52693] Do not construct memory accesses to unscalarizable regions

[Martin Jambor]

Hi,

this fixes a thinko that leads to wrong-code generation that is in the
"new" SRA since the beginning.  When there are two unscalarizable
regions in an access tree, one within another, the aggregate
assignment modification code may use them as basis of new memory
accesses, which means it very likely ignores an array reference along
the way.  Using such a region in the code path is useless anyway since
by its nature there cannot be any replacements there.

This is a patch for trunk (on which it has passed bootstrap and
testing on x86_64-linux) and the 4.7 branch, I'm in the process of
testing equivalents for the 4.5 and 4.6 branches (the diff contexts
differ slightly).  OK for everywhere if all tests pass?

Thanks,

Martin


2012-03-24  Martin Jambor  <mjambor@suse.cz>

	PR middle-end/52693
	* tree-sra.c (sra_modify_assign): Do not call
	load_assign_lhs_subreplacements when working with an unscalarizable
	region.

	* testsuite/gcc.dg/torture/pr52693.c: New test.


Index: src/gcc/tree-sra.c
===================================================================
--- src.orig/gcc/tree-sra.c
+++ src/gcc/tree-sra.c
@@ -3071,7 +3071,13 @@ sra_modify_assign (gimple *stmt, gimple_
     }
   else
     {
-      if (access_has_children_p (lacc) && access_has_children_p (racc))
+      if (access_has_children_p (lacc)
+	  && access_has_children_p (racc)
+	  /* When an access represents an unscalarizable region, it usually
+	     represents accesses with variable offset and thus must not be used
+	     to generate new memory accesses.  */
+	  && !lacc->grp_unscalarizable_region
+	  && !racc->grp_unscalarizable_region)
 	{
 	  gimple_stmt_iterator orig_gsi = *gsi;
 	  enum unscalarized_data_handling refreshed;
Index: src/gcc/testsuite/gcc.dg/torture/pr52693.c
===================================================================
--- /dev/null
+++ src/gcc/testsuite/gcc.dg/torture/pr52693.c
@@ -0,0 +1,33 @@
+/* { dg-do run } */
+
+struct pair
+{
+  int x;
+  int y;
+};
+
+struct array
+{
+  struct pair elems[ 2 ];
+  unsigned index;
+};
+
+extern void abort ();
+
+void __attribute__ ((noinline,noclone))
+test_results (int x1, int y1, int x2, int y2)
+{
+  if (x1 != x2 || y1 != y2)
+    abort ();
+}
+
+int
+main (void)
+{
+  struct array arr = {{{1,2}, {3,4}}, 1};
+  struct pair last = arr.elems[arr.index];
+
+  test_results ( last.x, last.y, arr.elems[1].x, arr.elems[1].y);
+
+  return 0;
+}

Re: [PATCH, PR 52693] Do not construct memory accesses to unscalarizable regions

[Richard Guenther]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2931 bytes --]

On Tue, 27 Mar 2012, Martin Jambor wrote:

> Hi,
> 
> this fixes a thinko that leads to wrong-code generation that is in the
> "new" SRA since the beginning.  When there are two unscalarizable
> regions in an access tree, one within another, the aggregate
> assignment modification code may use them as basis of new memory
> accesses, which means it very likely ignores an array reference along
> the way.  Using such a region in the code path is useless anyway since
> by its nature there cannot be any replacements there.
> 
> This is a patch for trunk (on which it has passed bootstrap and
> testing on x86_64-linux) and the 4.7 branch, I'm in the process of
> testing equivalents for the 4.5 and 4.6 branches (the diff contexts
> differ slightly).  OK for everywhere if all tests pass?

Ok.

Thanks,
Richard.

> Thanks,
> 
> Martin
> 
> 
> 2012-03-24  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR middle-end/52693
> 	* tree-sra.c (sra_modify_assign): Do not call
> 	load_assign_lhs_subreplacements when working with an unscalarizable
> 	region.
> 
> 	* testsuite/gcc.dg/torture/pr52693.c: New test.
> 
> 
> Index: src/gcc/tree-sra.c
> ===================================================================
> --- src.orig/gcc/tree-sra.c
> +++ src/gcc/tree-sra.c
> @@ -3071,7 +3071,13 @@ sra_modify_assign (gimple *stmt, gimple_
>      }
>    else
>      {
> -      if (access_has_children_p (lacc) && access_has_children_p (racc))
> +      if (access_has_children_p (lacc)
> +	  && access_has_children_p (racc)
> +	  /* When an access represents an unscalarizable region, it usually
> +	     represents accesses with variable offset and thus must not be used
> +	     to generate new memory accesses.  */
> +	  && !lacc->grp_unscalarizable_region
> +	  && !racc->grp_unscalarizable_region)
>  	{
>  	  gimple_stmt_iterator orig_gsi = *gsi;
>  	  enum unscalarized_data_handling refreshed;
> Index: src/gcc/testsuite/gcc.dg/torture/pr52693.c
> ===================================================================
> --- /dev/null
> +++ src/gcc/testsuite/gcc.dg/torture/pr52693.c
> @@ -0,0 +1,33 @@
> +/* { dg-do run } */
> +
> +struct pair
> +{
> +  int x;
> +  int y;
> +};
> +
> +struct array
> +{
> +  struct pair elems[ 2 ];
> +  unsigned index;
> +};
> +
> +extern void abort ();
> +
> +void __attribute__ ((noinline,noclone))
> +test_results (int x1, int y1, int x2, int y2)
> +{
> +  if (x1 != x2 || y1 != y2)
> +    abort ();
> +}
> +
> +int
> +main (void)
> +{
> +  struct array arr = {{{1,2}, {3,4}}, 1};
> +  struct pair last = arr.elems[arr.index];
> +
> +  test_results ( last.x, last.y, arr.elems[1].x, arr.elems[1].y);
> +
> +  return 0;
> +}
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer