This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Improve stack layout heuristic.


Hi,
 This patch impoves the heuristic used in assigning stack location to
stack variables.  Currently, if there are 3 variables A, B and C with
their sizes in increasing order and A and C have a conflict, it will
put A and B in a partition and C in a separate partition with a total
frame size of sizeof(B) + sizeof(C).  This patch puts B and C in the
same partition and A in a separate partition, with a total size of
sizeof(A) + sizeof(C).  The other improvement is when
-fno-strict-aliasing is used. In that case, it is ok to share stack
slot for a variable whose type transitively contains a union. The
other change in this patch removes the field offset in struct
stack_var. Right now, the field is always 0 due to the way it is set
in partition_stack_vars.

Bootstraps and no test regressions on x86_64-unknown-linux. OK for trunk?

-Easwaran


2011-04-17  Easwaran Raman  <eraman@google.com>

	* gcc/testsuite/gcc.dg/stack-layout-1.c: New test.
	* gcc/testsuite/gcc.dg/stack-layout-2.c: New test.
	* gcc/cfgexpand.c (struct stack_var): Remove OFFSET...
	(add_stack_var): ...and its reference here...
	(expand_stack_vars): ...and here.
	(add_alias_set_conflicts): Add conflict to union variables
	only when strict aliasing is used.
	(stack_var_cmp): Sort by descending order of size.
	(partition_stack_vars): Change heuristic.
	(union_stack_vars): Fix to refelect changes in
	partition_stack_vars.
	(dump_stack_var_partition): Add newline after each partition.
Index: gcc/testsuite/gcc.dg/stack-layout-1.c
===================================================================
--- gcc/testsuite/gcc.dg/stack-layout-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/stack-layout-1.c	(revision 0)
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-strict-aliasing -fdump-rtl-expand" } */
+union U {
+  int a;
+  float b;
+};
+struct A {
+  union U u1;
+  char a[100];
+};
+void bar (struct A *);
+void foo ()
+  {
+    {
+      struct A a;
+      bar (&a);
+    }
+    {
+      struct A a;
+      bar (&a);
+    }
+  }
+
+/* { dg-final { scan-rtl-dump-times "Partition" 1 "expand" } } */
+/* { dg-final { cleanup-rtl-dump "expand" } } */
Index: gcc/testsuite/gcc.dg/stack-layout-2.c
===================================================================
--- gcc/testsuite/gcc.dg/stack-layout-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/stack-layout-2.c	(revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-rtl-expand" } */
+void bar( char *);
+int foo()
+{
+  int i=0;
+  {
+    char a[8000];
+    bar(a);
+    i += a[0];
+  }
+  {
+    char a[8192];
+    char b[32];
+    bar(a);
+    i += a[0];
+    bar(b);
+    i += a[0];
+  }
+  return i;
+}
+/* { dg-final { scan-rtl-dump "size 8192" "expand" } } */
+/* { dg-final { scan-rtl-dump "size 32" "expand" } } */
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	(revision 171954)
+++ gcc/cfgexpand.c	(working copy)
@@ -158,11 +158,6 @@
   /* The Variable.  */
   tree decl;

-  /* The offset of the variable.  During partitioning, this is the
-     offset relative to the partition.  After partitioning, this
-     is relative to the stack frame.  */
-  HOST_WIDE_INT offset;
-
   /* Initially, the size of the variable.  Later, the size of the partition,
      if this variable becomes it's partition's representative.  */
   HOST_WIDE_INT size;
@@ -267,7 +262,6 @@
   v = &stack_vars[stack_vars_num];

   v->decl = decl;
-  v->offset = 0;
   v->size = tree_low_cst (DECL_SIZE_UNIT (SSAVAR (decl)), 1);
   /* Ensure that all variables have size, so that &a != &b for any two
      variables that are simultaneously live.  */
@@ -372,8 +366,9 @@
 		 to elements will conflict.  In case of unions we have
 		 to be careful as type based aliasing rules may say
 		 access to the same memory does not conflict.  So play
-		 safe and add a conflict in this case.  */
-	      || contains_union)
+		 safe and add a conflict in this case when -fstrict-aliasing
+                 is used.  */
+	      || (contains_union && flag_strict_aliasing))
 	    add_stack_var_conflict (i, j);
 	}
     }
@@ -403,9 +398,9 @@
     return (int)largeb - (int)largea;

   /* Secondary compare on size, decreasing  */
-  if (sizea < sizeb)
-    return -1;
   if (sizea > sizeb)
+    return -1;
+  if (sizea < sizeb)
     return 1;

   /* Tertiary compare on true alignment, decreasing.  */
@@ -564,28 +559,18 @@

 /* A subroutine of partition_stack_vars.  The UNION portion of a UNION/FIND
    partitioning algorithm.  Partitions A and B are known to be non-conflicting.
-   Merge them into a single partition A.
+   Merge them into a single partition A.  */

-   At the same time, add OFFSET to all variables in partition B.  At the end
-   of the partitioning process we've have a nice block easy to lay out within
-   the stack frame.  */
-
 static void
-union_stack_vars (size_t a, size_t b, HOST_WIDE_INT offset)
+union_stack_vars (size_t a, size_t b)
 {
-  size_t i, last;
   struct stack_var *vb = &stack_vars[b];
   bitmap_iterator bi;
   unsigned u;

-  /* Update each element of partition B with the given offset,
-     and merge them into partition A.  */
-  for (last = i = b; i != EOC; last = i, i = stack_vars[i].next)
-    {
-      stack_vars[i].offset += offset;
-      stack_vars[i].representative = a;
-    }
-  stack_vars[last].next = stack_vars[a].next;
+   /* Add B to A's partition.  */
+  stack_vars[b].next = stack_vars[a].next;
+  stack_vars[b].representative = a;
   stack_vars[a].next = b;

   /* Update the required alignment of partition A to account for B.  */
@@ -596,7 +581,7 @@
   if (vb->conflicts)
     {
       EXECUTE_IF_SET_IN_BITMAP (vb->conflicts, 0, u, bi)
-	add_stack_var_conflict (a, stack_vars[u].representative);
+	add_stack_var_conflict (a, u);
       BITMAP_FREE (vb->conflicts);
     }
 }
@@ -605,16 +590,13 @@
    partitions constrained by the interference graph.  The overall
    algorithm used is as follows:

-	Sort the objects by size.
+	Sort the objects by size in descending order.
 	For each object A {
 	  S = size(A)
 	  O = 0
 	  loop {
 	    Look for the largest non-conflicting object B with size <= S.
 	    UNION (A, B)
-	    offset(B) = O
-	    O += size(B)
-	    S -= size(B)
 	  }
 	}
 */
@@ -636,24 +618,23 @@
   for (si = 0; si < n; ++si)
     {
       size_t i = stack_vars_sorted[si];
-      HOST_WIDE_INT isize = stack_vars[i].size;
       unsigned int ialign = stack_vars[i].alignb;
-      HOST_WIDE_INT offset = 0;

-      for (sj = si; sj-- > 0; )
+      /* Ignore objects that aren't partition representatives. If we
+         see a var that is not a partition representative, it must
+         have been merged earlier.  */
+      if (stack_vars[i].representative != i)
+        continue;
+
+      for (sj = si + 1; sj < n; ++sj)
 	{
 	  size_t j = stack_vars_sorted[sj];
-	  HOST_WIDE_INT jsize = stack_vars[j].size;
 	  unsigned int jalign = stack_vars[j].alignb;

 	  /* Ignore objects that aren't partition representatives.  */
 	  if (stack_vars[j].representative != j)
 	    continue;

-	  /* Ignore objects too large for the remaining space.  */
-	  if (isize < jsize)
-	    continue;
-
 	  /* Ignore conflicting objects.  */
 	  if (stack_var_conflict_p (i, j))
 	    continue;
@@ -664,25 +645,8 @@
 	      != (jalign * BITS_PER_UNIT <= MAX_SUPPORTED_STACK_ALIGNMENT))
 	    continue;

-	  /* Refine the remaining space check to include alignment.  */
-	  if (offset & (jalign - 1))
-	    {
-	      HOST_WIDE_INT toff = offset;
-	      toff += jalign - 1;
-	      toff &= -(HOST_WIDE_INT)jalign;
-	      if (isize - (toff - offset) < jsize)
-		continue;
-
-	      isize -= toff - offset;
-	      offset = toff;
-	    }
-
 	  /* UNION the objects, placing J at OFFSET.  */
-	  union_stack_vars (i, j, offset);
-
-	  isize -= jsize;
-	  if (isize == 0)
-	    break;
+	  union_stack_vars (i, j);
 	}
     }

@@ -712,9 +676,8 @@
 	{
 	  fputc ('\t', dump_file);
 	  print_generic_expr (dump_file, stack_vars[j].decl, dump_flags);
-	  fprintf (dump_file, ", offset " HOST_WIDE_INT_PRINT_DEC "\n",
-		   stack_vars[j].offset);
 	}
+      fputc ('\n', dump_file);
     }
 }

@@ -863,10 +826,9 @@
 	 partition.  */
       for (j = i; j != EOC; j = stack_vars[j].next)
 	{
-	  gcc_assert (stack_vars[j].offset <= stack_vars[i].size);
 	  expand_one_stack_var_at (stack_vars[j].decl,
 				   base, base_align,
-				   stack_vars[j].offset + offset);
+				   offset);
 	}
     }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]