[PATCH] (another one) Fix PR33870, PTA and partitoning interaction

Richard Guenther rguenther@suse.de
Fri Oct 26 14:54:00 GMT 2007


On Wed, 24 Oct 2007, Daniel Berlin wrote:

> On 10/24/07, Richard Guenther <rguenther@suse.de> wrote:
> > On Wed, 24 Oct 2007, Daniel Berlin wrote:
> >
> > > On 10/24/07, Richard Guenther <rguenther@suse.de> wrote:
> > > >
> > > > So, as opposed to the earlier patch which papered over one specific case
> > > > of the problem (which is that alias partitioning rips apart SFTs belonging
> > > > to the same parent variable), this is a hammer that really solves it
> > > > but at a cost (maybe, to be investigated).
> > > >
> > > > IMHO the operand scanning part of accesses through pointers is fragile
> > > > in that it requires that the full set of SFTs that possibly are addressed
> > > > by the pointer is available together as aliases of a symbol.
> > >
> > > Actually, it only requires that the points-to sets be correct :)
> >
> > But we don't use the points-to set.  We use the aliased bitmap.
> 
> Feel free to keep the ptset around for 4.3 (i think we already do),
> and just use that.
> I'd happily approve that.

I tried and it's not too easy.  But the following works and should do
the equivalent thing by looking into MPTs.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Ok if that passes?

Richard.


2007-10-26  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/33870
	* tree-ssa-operands.c (add_vars_for_offset): Reduce code
	duplication.  Remove redundant call to access_can_touch_variable.
	(add_vars_for_bitmap): New helper for recursing over MPT contents.
	(add_virtual_operand): Use it.

	* gcc.dg/tree-ssa/alias-15.c: New testcase.
	* gcc.c-torture/execute/pr33870.c: Likewise.

Index: tree-ssa-operands.c
===================================================================
--- tree-ssa-operands.c	(revision 129646)
+++ tree-ssa-operands.c	(working copy)
@@ -1419,55 +1419,57 @@ add_vars_for_offset (tree full_ref, tree
     }
   else if (TREE_CODE (var) == STRUCT_FIELD_TAG)
     {      
-      if (size == -1)
+      bool added = false;
+      subvar_t sv = get_subvars_for_var (SFT_PARENT_VAR (var));
+      for (; sv; sv = sv->next)
 	{
-	  bool added = false;
-	  subvar_t sv = get_subvars_for_var (SFT_PARENT_VAR (var));
-	  for (; sv; sv = sv->next)
-	    {
-	      if (overlap_subvar (SFT_OFFSET (var) + offset, size,
-				  sv->var, NULL)
-		  && access_can_touch_variable (full_ref, sv->var,
-						offset, size))
-		{
-		  added = true;
-		  if (is_def)
-		    append_vdef (sv->var);
-		  else
-		    append_vuse (sv->var);
-		}
-	    }
-	  return added;
-	}
-      else
-	{
-	  bool added = false;
-	  subvar_t sv = get_subvars_for_var (SFT_PARENT_VAR (var));
-	  for (; sv; sv = sv->next)
+	  /* Once we hit the end of the parts that could touch,
+	     stop looking.  */
+	  if (size != -1
+	      && SFT_OFFSET (var) + offset + size <= SFT_OFFSET (sv->var))
+	    break;
+	  if (overlap_subvar (SFT_OFFSET (var) + offset, size, sv->var, NULL))
 	    {
-	      /* Once we hit the end of the parts that could touch,
-		 stop looking.  */
-	      if (SFT_OFFSET (var) + offset + size <= SFT_OFFSET (sv->var))
-		break;
-	      if (overlap_subvar (SFT_OFFSET (var) + offset, size,
-				  sv->var, NULL)
-		  && access_can_touch_variable (full_ref, sv->var, offset, 
-						size))
-		{
-		  added = true;
-		  if (is_def)
-		    append_vdef (sv->var);
-		  else
-		    append_vuse (sv->var);
-		}
+	      added = true;
+	      if (is_def)
+		append_vdef (sv->var);
+	      else
+		append_vuse (sv->var);
 	    }
-	  return added;
 	}
+      return added;
     }
   
   return false;
 }
 
+/* Add all aliases from ALIASES as virtual operands for the access
+   FULL_REF at OFFSET and size SIZE.  IS_CALL_SITE is true if the
+   stmt of the reference is a call.  IS_DEF is true if we should add
+   VDEF virtual operands, otherwise we'll add VUSEs.  *NONE_ADDED
+   is set to false once the first virtual operand was added.  */
+
+static void
+add_vars_for_bitmap (bitmap aliases, tree full_ref,
+		     HOST_WIDE_INT offset, HOST_WIDE_INT size,
+		     bool is_call_site, bool is_def, bool *none_added)
+{
+  bitmap_iterator bi;
+  unsigned int i;
+
+  EXECUTE_IF_SET_IN_BITMAP (aliases, 0, i, bi)
+    {
+      tree al = referenced_var (i);
+
+      if (TREE_CODE (al) == MEMORY_PARTITION_TAG)
+	add_vars_for_bitmap (MPT_SYMBOLS (al), full_ref,
+			     offset, size, is_call_site, is_def, none_added);
+      else
+	*none_added &= !add_vars_for_offset (full_ref, al, offset, size,
+					     is_call_site, is_def);
+    }
+}
+
 /* Add VAR to the virtual operands array.  FLAGS is as in
    get_expr_operands.  FULL_REF is a tree that contains the entire
    pointer dereference expression, if available, or NULL otherwise.
@@ -1530,24 +1532,17 @@ add_virtual_operand (tree var, stmt_ann_
     }
   else
     {
-      bitmap_iterator bi;
-      unsigned int i;
-      tree al;
+      bool none_added = true;
       
       /* The variable is aliased.  Add its aliases to the virtual
 	 operands.  */
       gcc_assert (!bitmap_empty_p (aliases));
-      
+
+      add_vars_for_bitmap (aliases, full_ref, offset, size,
+			   is_call_site, flags & opf_def, &none_added);
+
       if (flags & opf_def)
 	{
-	  bool none_added = true;
-	  EXECUTE_IF_SET_IN_BITMAP (aliases, 0, i, bi)
-	    {
-	      al = referenced_var (i);
-	      none_added &= !add_vars_for_offset (full_ref, al, offset, size,
-						  is_call_site, true);
-	    }
-
 	  /* If the variable is also an alias tag, add a virtual
 	     operand for it, otherwise we will miss representing
 	     references to the members of the variable's alias set.	     
@@ -1566,15 +1561,6 @@ add_virtual_operand (tree var, stmt_ann_
 	}
       else
 	{
-	  bool none_added = true;
-	  EXECUTE_IF_SET_IN_BITMAP (aliases, 0, i, bi)
-	    {
-	      al = referenced_var (i);
-	      none_added &= !add_vars_for_offset (full_ref, al, offset, size,
-						  is_call_site, false);
-	      
-	    }
-	  
 	  /* Even if no aliases have been added, we still need to
 	     establish def-use and use-def chains, lest
 	     transformations think that this is not a memory
Index: testsuite/gcc.c-torture/execute/pr33870.c
===================================================================
*** testsuite/gcc.c-torture/execute/pr33870.c	(revision 0)
--- testsuite/gcc.c-torture/execute/pr33870.c	(revision 0)
***************
*** 0 ****
--- 1,87 ----
+ extern void abort (void);
+ 
+ typedef struct PgHdr PgHdr;
+ typedef unsigned char u8;
+ struct PgHdr {
+   unsigned int pgno;
+   PgHdr *pNextHash, *pPrevHash;
+   PgHdr *pNextFree, *pPrevFree;
+   PgHdr *pNextAll;
+   u8 inJournal;
+   short int nRef;
+   PgHdr *pDirty, *pPrevDirty;
+   unsigned int notUsed;
+ };
+ 
+ static inline PgHdr *merge_pagelist(PgHdr *pA, PgHdr *pB)
+ {
+   PgHdr result;
+   PgHdr *pTail;
+   pTail = &result;
+   while( pA && pB ){
+     if( pA->pgno<pB->pgno ){
+       pTail->pDirty = pA;
+       pTail = pA;
+       pA = pA->pDirty;
+     }else{
+       pTail->pDirty = pB;
+       pTail = pB;
+       pB = pB->pDirty;
+     }
+   }
+   if( pA ){
+     pTail->pDirty = pA;
+   }else if( pB ){
+     pTail->pDirty = pB;
+   }else{
+     pTail->pDirty = 0;
+   }
+   return result.pDirty;
+ }
+ 
+ PgHdr * __attribute__((noinline)) sort_pagelist(PgHdr *pIn)
+ {
+   PgHdr *a[25], *p;
+   int i;
+   __builtin_memset (a, 0, sizeof (a));
+   while( pIn ){
+     p = pIn;
+     pIn = p->pDirty;
+     p->pDirty = 0;
+     for(i=0; i<25 -1; i++){
+       if( a[i]==0 ){
+         a[i] = p;
+         break;
+       }else{
+         p = merge_pagelist(a[i], p);
+         a[i] = 0;
+       }
+     }
+     if( i==25 -1 ){
+       a[i] = merge_pagelist(a[i], p);
+     }
+   }
+   p = a[0];
+   for(i=1; i<25; i++){
+     p = merge_pagelist (p, a[i]);
+   }
+   return p;
+ }
+ 
+ int main()
+ {
+   PgHdr a[5];
+   PgHdr *p;
+   a[0].pgno = 5;
+   a[0].pDirty = &a[1];
+   a[1].pgno = 4;
+   a[1].pDirty = &a[2];
+   a[2].pgno = 1;
+   a[2].pDirty = &a[3];
+   a[3].pgno = 3;
+   a[3].pDirty = 0;
+   p = sort_pagelist (&a[0]);
+   if (p->pDirty == p)
+     abort ();
+   return 0;
+ }
Index: testsuite/gcc.dg/tree-ssa/alias-15.c
===================================================================
*** testsuite/gcc.dg/tree-ssa/alias-15.c	(revision 0)
--- testsuite/gcc.dg/tree-ssa/alias-15.c	(revision 0)
***************
*** 0 ****
--- 1,20 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O -fno-early-inlining -fdump-tree-salias-vops-details" } */
+ 
+ struct foo {
+   int a;
+   struct X {
+     int b[4];
+   } b;
+ } m;
+ static inline struct X *wrap(struct X *p) { return p; }
+ int test2(void)
+ {
+   struct X *p = wrap(&m.b);
+   /* Both memory references need to alias the same SFT.  */
+   return p->b[3] - m.b.b[3];
+ }
+ 
+ /* { dg-final { scan-tree-dump "SFT.1 created for var m offset 128" "salias" } } */
+ /* { dg-final { scan-tree-dump-times "VUSE <SFT.1_" 2 "salias" } } */
+ /* { dg-final { cleanup-tree-dump "salias" } } */



More information about the Gcc-patches mailing list