[PATCH] Fix PR rtl-optimization/25654 (P1 4.0/4.1/4.2 Regression)

[Richard Guenther]

On Mon, 16 Jan 2006, Richard Guenther wrote:

> This patch fixes the stack slot sharing aliasing problem by making sure
> to add conflicts for unions of the same type.
> 
> Bootstrapped and regtested on x86_64-unknown-linux-gnu.
> 
> Ok for mainline and branches?

Honza still figures out more failing testcases, this patch
fixes two of them while leaving the following (invalid because
of aliasing issues char vs. int/short) failing:

-O2 -mpreferred-stack-boundary=2 -S -march=i686 -frename-registers

struct setconflict
{
  char a[36];
  int b;
};

int
main ()
{
  int sum = 0;
  {
    struct setconflict a;
    short *c;
    c = (void *)a.a;
    asm ("": "=r" (c):"0" (c));
    *c = 0;
    asm ("": "=r" (c):"0" (c));
    sum += *c;
  }
  {
    struct setconflict a;
    int *c;
    c = (void *)a.a;
    asm ("": "=r" (c):"0" (c));
    *c = 1;
    asm ("": "=r" (c):"0" (c));
    sum += *c;
  }

  printf ("%d\n",sum);
  if (sum != 1)
    abort();
  return 0;
}

Does this look invalid to anyone else?

Richard.


2006-01-16  Steven Bosscher  <stevenb.gcc@gmail.com>
	Jan Hubicka  <jh@suse.cz>
	Richard Guenther  <rguenther@suse.de>

	PR rtl-optimization/25654
	* cfgexpand.c (stack_var_conflict_p): Make sure to add conflicts
	for variables with the same union type.

	* gcc.dg/torture/pr25654.c: New testcase.
	* gcc.target/i386/pr25654-3.c: Likewise.

Index: cfgexpand.c
===================================================================
*** cfgexpand.c	(revision 109743)
--- cfgexpand.c	(working copy)
*************** stack_var_conflict_p (size_t x, size_t y
*** 272,281 ****
    gcc_assert (index < stack_vars_conflict_alloc);
    return stack_vars_conflict[index];
  }
!   
  /* A subroutine of expand_used_vars.  If two variables X and Y have alias
     sets that do not conflict, then do add a conflict for these variables
!    in the interference graph.  We also have to mind MEM_IN_STRUCT_P and
     MEM_SCALAR_P.  */
  
  static void
--- 272,305 ----
    gcc_assert (index < stack_vars_conflict_alloc);
    return stack_vars_conflict[index];
  }
!  
! /* Returns true if TYPE is or contains a union type.  */
! 
! static bool
! aggregate_contains_union_type (tree type)
! {
!   tree field;
! 
!   if (TREE_CODE (type) == UNION_TYPE
!       || TREE_CODE (type) == QUAL_UNION_TYPE)
!     return true;
!   if (TREE_CODE (type) == ARRAY_TYPE)
!     return aggregate_contains_union_type (TREE_TYPE (type));
!   if (TREE_CODE (type) != RECORD_TYPE)
!     return false;
! 
!   for (field = TYPE_FIELDS (type); field; field = TREE_CHAIN (field))
!     if (TREE_CODE (field) == FIELD_DECL)
!       if (aggregate_contains_union_type (TREE_TYPE (field)))
! 	return true;
! 
!   return false;
! }
! 
  /* A subroutine of expand_used_vars.  If two variables X and Y have alias
     sets that do not conflict, then do add a conflict for these variables
!    in the interference graph.  We also need to make sure to add conflicts
!    for unions of the same type.  We also have to mind MEM_IN_STRUCT_P and
     MEM_SCALAR_P.  */
  
  static void
*************** add_alias_set_conflicts (void)
*** 292,298 ****
  	{
  	  tree type_j = TREE_TYPE (stack_vars[j].decl);
  	  bool aggr_j = AGGREGATE_TYPE_P (type_j);
! 	  if (aggr_i != aggr_j || !objects_must_conflict_p (type_i, type_j))
  	    add_stack_var_conflict (i, j);
  	}
      }
--- 316,325 ----
  	{
  	  tree type_j = TREE_TYPE (stack_vars[j].decl);
  	  bool aggr_j = AGGREGATE_TYPE_P (type_j);
! 	  if (aggr_i != aggr_j
! 	      || !objects_must_conflict_p (type_i, type_j)
! 	      || (aggregate_contains_union_type (type_i)
! 		  && type_i == type_j))
  	    add_stack_var_conflict (i, j);
  	}
      }
Index: testsuite/gcc.dg/torture/pr25654.c
===================================================================
*** testsuite/gcc.dg/torture/pr25654.c	(revision 0)
--- testsuite/gcc.dg/torture/pr25654.c	(revision 0)
***************
*** 0 ****
--- 1,37 ----
+ /* { dg-do run } */
+ 
+ extern void abort (void) __attribute__((noreturn));
+ 
+ union setconflict
+ {
+   short a[20];
+   int b[10];
+ };
+ 
+ int
+ main ()
+ {
+   int sum = 0;
+   {
+     union setconflict a;
+     short *c;
+     c = a.a;
+     asm ("": "=r" (c):"0" (c));
+     *c = 0;
+     asm ("": "=r" (c):"0" (c));
+     sum += *c;
+   }
+   {
+     union setconflict a;
+     int *c;
+     c = a.b;
+     asm ("": "=r" (c):"0" (c));
+     *c = 1;
+     asm ("": "=r" (c):"0" (c));
+     sum += *c;
+   }
+ 
+   if (sum != 1)
+     abort();
+   return 0;
+ }
Index: testsuite/gcc.target/i386/pr25654-3.c
===================================================================
*** testsuite/gcc.target/i386/pr25654-3.c	(revision 0)
--- testsuite/gcc.target/i386/pr25654-3.c	(revision 0)
***************
*** 0 ****
--- 1,41 ----
+ /* { dg-do run } */
+ /* { dg-options "-O2 -mpreferred-stack-boundary=2 -march=i686 -frename-registers" } */
+ 
+ extern void abort (void) __attribute__((noreturn));
+ 
+ struct wrapper {
+ union setconflict
+ {
+   short a[20];
+   int b[10];
+ } a;
+ };
+ 
+ int
+ main ()
+ {
+   int sum = 0;
+   {
+     struct wrapper a;
+     short *c;
+     c = a.a.a;
+     asm ("": "=r" (c):"0" (c));
+     *c = 0;
+     asm ("": "=r" (c):"0" (c));
+     sum += *c;
+   }
+   {
+     struct wrapper a;
+     int *c;
+     c = a.a.b;
+     asm ("": "=r" (c):"0" (c));
+     *c = 1;
+     asm ("": "=r" (c):"0" (c));
+     sum += *c;
+   }
+ 
+   if (sum != 1)
+     abort();
+   return 0;
+ }
+