Patchwork loop-unroll.c TLC 3/4 simple peeling heuristic fix

login
register
mail settings
Submitter Jan Hubicka
Date Oct. 23, 2012, 3:18 p.m.
Message ID <20121023151825.GB5020@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/193504/
State New
Headers show

Comments

Jan Hubicka - Oct. 23, 2012, 3:18 p.m.
Hi,
simple peeling heuristic thinks it makes no sense to peel loops with known
iteration count (because they will be runtime unrolled instead).  This is not
true because the known iteration count is only upper bound. Fixed this.
To make testcase possible I had to reduce overactive heuristic on number of
branches in the loop.  It looks bit more like an thinko copied from simple
unrolling where it makes sort of more sense.

Peeling first iterations when loop is known to execute few times makes
sense for branch prediction queality.

Bootstrapped/regtested x86_64-linux, comitted.

Honza

Patch

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 192717)
+++ ChangeLog	(working copy)
@@ -1,3 +1,11 @@ 
+2012-10-23  Jan Hubicka  <jh@suse.cz>
+
+	* loop-unroll.c (decide_peel_simple): Simple peeling makes sense even
+	with simple loops; bound number of branches only when FDO is not
+	available.
+	(decide_unroll_stupid): Mention that num_loop_branches heuristics
+	is off.
+
 2012-10-23  Nick Clifton  <nickc@redhat.com>
 
 	PR target/54660
Index: loop-unroll.c
===================================================================
--- loop-unroll.c	(revision 192717)
+++ loop-unroll.c	(working copy)
@@ -1228,7 +1228,6 @@  static void
 decide_peel_simple (struct loop *loop, int flags)
 {
   unsigned npeel;
-  struct niter_desc *desc;
   double_int iterations;
 
   if (!(flags & UAP_PEEL))
@@ -1253,20 +1252,17 @@  decide_peel_simple (struct loop *loop, i
       return;
     }
 
-  /* Check for simple loops.  */
-  desc = get_simple_loop_desc (loop);
-
-  /* Check number of iterations.  */
-  if (desc->simple_p && !desc->assumptions && desc->const_iter)
-    {
-      if (dump_file)
-	fprintf (dump_file, ";; Loop iterates constant times\n");
-      return;
-    }
-
   /* Do not simply peel loops with branches inside -- it increases number
-     of mispredicts.  */
-  if (num_loop_branches (loop) > 1)
+     of mispredicts.  
+     Exception is when we do have profile and we however have good chance
+     to peel proper number of iterations loop will iterate in practice.
+     TODO: this heuristic needs tunning; while for complette unrolling
+     the branch inside loop mostly eliminates any improvements, for
+     peeling it is not the case.  Also a function call inside loop is
+     also branch from branch prediction POV (and probably better reason
+     to not unroll/peel).  */
+  if (num_loop_branches (loop) > 1
+      && profile_status != PROFILE_READ)
     {
       if (dump_file)
 	fprintf (dump_file, ";; Not peeling, contains branches\n");
@@ -1435,7 +1431,9 @@  decide_unroll_stupid (struct loop *loop,
     }
 
   /* Do not unroll loops with branches inside -- it increases number
-     of mispredicts.  */
+     of mispredicts. 
+     TODO: this heuristic needs tunning; call inside the loop body
+     is also relatively good reason to not unroll.  */
   if (num_loop_branches (loop) > 1)
     {
       if (dump_file)
Index: testsuite/gcc.dg/tree-prof/peel-1.c
===================================================================
--- testsuite/gcc.dg/tree-prof/peel-1.c	(revision 0)
+++ testsuite/gcc.dg/tree-prof/peel-1.c	(revision 0)
@@ -0,0 +1,25 @@ 
+/* { dg-options "-O3 -fdump-rtl-loop2_unroll -fno-unroll-loops -fpeel-loops" } */
+void abort();
+
+int a[1000];
+int
+__attribute__ ((noinline))
+t()
+{
+  int i;
+  for (i=0;i<1000;i++)
+    if (!a[i])
+      return 1;
+  abort ();
+}
+main()
+{
+  int i;
+  for (i=0;i<1000;i++)
+    t();
+  return 0;
+}
+/* { dg-final-use { scan-rtl-dump "Considering simply peeling loop" "loop2_unroll" } } */
+/* In fact one peeling is enough; we however mispredict number of iterations of the loop
+   at least until loop_ch is schedule ahead of profiling pass.  */
+/* { dg-final-use { cleanup-rtl-dump "Decided to simply peel the loop 2 times" } } */
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog	(revision 192717)
+++ testsuite/ChangeLog	(working copy)
@@ -1,7 +1,11 @@ 
+2012-10-23  Jan Hubicka  <jh@suse.cz>
+
+	* gcc.dg/tree-prof/peel-1.c: New testcase.
+
 2012-10-23  Dominique d'Humieres  <dominiq@lps.ens.fr>
 
 	PR gcc/52945
-	* testsuite/gcc.dg/lto/pr52634_0.c: skip the test on Darwin.
+	* gcc.dg/lto/pr52634_0.c: skip the test on Darwin.
 
 2012-10-23  Joseph Myers  <joseph@codesourcery.com>