diff mbox

Introduce param for copy loop headers pass

Message ID 20160608093113.GA79695@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka June 8, 2016, 9:31 a.m. UTC
Hi,
I think 20 insns to copy for loop header is way too much. The constant came
from jump.c that was operating with quite different IL and compiler.
This patch adds --param for it so we can fine tune it for new millenia.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* invoke.texi (max-loop-headers-insns): Document.
	* params.def (PARAM_MAX_LOOP_HEADER_INSNS): New.
	* tree-ssa-loop-ch.c (should_duplicate_loop_header_p): Update comment.
	(ch_base::copy_headers): Use PARAM_MAX_LOOP_HEADER_INSNS.

Comments

Richard Biener June 8, 2016, 11:29 a.m. UTC | #1
On Wed, 8 Jun 2016, Jan Hubicka wrote:

> Hi,
> I think 20 insns to copy for loop header is way too much. The constant came
> from jump.c that was operating with quite different IL and compiler.
> This patch adds --param for it so we can fine tune it for new millenia.
> 
> Bootstrapped/regtested x86_64-linux, OK?

Ok.

Thanks
Richard.

> Honza
> 
> 	* invoke.texi (max-loop-headers-insns): Document.
> 	* params.def (PARAM_MAX_LOOP_HEADER_INSNS): New.
> 	* tree-ssa-loop-ch.c (should_duplicate_loop_header_p): Update comment.
> 	(ch_base::copy_headers): Use PARAM_MAX_LOOP_HEADER_INSNS.
> 
> Index: doc/invoke.texi
> ===================================================================
> --- doc/invoke.texi	(revision 237184)
> +++ doc/invoke.texi	(working copy)
> @@ -9066,6 +9066,9 @@ The maximum number of insns of an unswit
>  @item max-unswitch-level
>  The maximum number of branches unswitched in a single loop.
>  
> +@item max-loop-headers-insns
> +The maximum number of insns in loop header duplicated by copy loop headers pass.
> +
>  @item lim-expensive
>  The minimum cost of an expensive expression in the loop invariant motion.
>  
> Index: params.def
> ===================================================================
> --- params.def	(revision 237184)
> +++ params.def	(working copy)
> @@ -344,6 +344,13 @@ DEFPARAM(PARAM_MAX_UNSWITCH_LEVEL,
>  	"The maximum number of unswitchings in a single loop.",
>  	3, 0, 0)
>  
> +/* The maximum number of insns in loop header duplicated by copy loop headers
> +   pass.  */
> +DEFPARAM(PARAM_MAX_LOOP_HEADER_INSNS,
> +	"max-loop-header-insns",
> +	"The maximum number of insns in loop header duplicated by copy loop headers pass.",
> +	20, 0, 0)
> +
>  /* The maximum number of iterations of a loop the brute force algorithm
>     for analysis of # of iterations of the loop tries to evaluate.  */
>  DEFPARAM(PARAM_MAX_ITERATIONS_TO_TRACK,
> Index: tree-ssa-loop-ch.c
> ===================================================================
> --- tree-ssa-loop-ch.c	(revision 237184)
> +++ tree-ssa-loop-ch.c	(working copy)
> @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.
>  #include "tree-inline.h"
>  #include "tree-ssa-scopedtables.h"
>  #include "tree-ssa-threadedge.h"
> +#include "params.h"
>  
>  /* Duplicates headers of loops if they are small enough, so that the statements
>     in the loop body are always executed when the loop is entered.  This
> @@ -106,8 +107,7 @@ should_duplicate_loop_header_p (basic_bl
>        return false;
>      }
>  
> -  /* Approximately copy the conditions that used to be used in jump.c --
> -     at most 20 insns and no calls.  */
> +  /* Count number of instructions and punt on calls.  */
>    for (bsi = gsi_start_bb (header); !gsi_end_p (bsi); gsi_next (&bsi))
>      {
>        last = gsi_stmt (bsi);
> @@ -290,8 +290,8 @@ ch_base::copy_headers (function *fun)
>  
>    FOR_EACH_LOOP (loop, 0)
>      {
> -      /* Copy at most 20 insns.  */
> -      int limit = 20;
> +      int ninsns = PARAM_VALUE (PARAM_MAX_LOOP_HEADER_INSNS);
> +      int limit = ninsns;
>        if (dump_file && (dump_flags & TDF_DETAILS))
>  	fprintf (dump_file,
>  		 "Analyzing loop %i\n", loop->num);
> @@ -333,7 +333,8 @@ ch_base::copy_headers (function *fun)
>  	fprintf (dump_file,
>  		 "Duplicating header of the loop %d up to edge %d->%d,"
>  		 " %i insns.\n",
> -		 loop->num, exit->src->index, exit->dest->index, 20 - limit);
> +		 loop->num, exit->src->index, exit->dest->index,
> +		 ninsns - limit);
>  
>        /* Ensure that the header will have just the latch as a predecessor
>  	 inside the loop.  */
> 
>
Bernd Schmidt June 8, 2016, 11:29 a.m. UTC | #2
On 06/08/2016 11:31 AM, Jan Hubicka wrote:
> I think 20 insns to copy for loop header is way too much. The constant came
> from jump.c that was operating with quite different IL and compiler.
> This patch adds --param for it so we can fine tune it for new millenia.

> +@item max-loop-headers-insns
> +The maximum number of insns in loop header duplicated by copy loop headers pass.
> +

"the copy loop headers pass", here and in params.def.

> -      int limit = 20;
> +      int ninsns = PARAM_VALUE (PARAM_MAX_LOOP_HEADER_INSNS);
> +      int limit = ninsns;

The naming is somewhat unfortunate, I think limit should be the initial 
limit, and something like remaining_limit should be the name of the one 
that counts down.

Otherwise ok.


Bernd
diff mbox

Patch

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 237184)
+++ doc/invoke.texi	(working copy)
@@ -9066,6 +9066,9 @@  The maximum number of insns of an unswit
 @item max-unswitch-level
 The maximum number of branches unswitched in a single loop.
 
+@item max-loop-headers-insns
+The maximum number of insns in loop header duplicated by copy loop headers pass.
+
 @item lim-expensive
 The minimum cost of an expensive expression in the loop invariant motion.
 
Index: params.def
===================================================================
--- params.def	(revision 237184)
+++ params.def	(working copy)
@@ -344,6 +344,13 @@  DEFPARAM(PARAM_MAX_UNSWITCH_LEVEL,
 	"The maximum number of unswitchings in a single loop.",
 	3, 0, 0)
 
+/* The maximum number of insns in loop header duplicated by copy loop headers
+   pass.  */
+DEFPARAM(PARAM_MAX_LOOP_HEADER_INSNS,
+	"max-loop-header-insns",
+	"The maximum number of insns in loop header duplicated by copy loop headers pass.",
+	20, 0, 0)
+
 /* The maximum number of iterations of a loop the brute force algorithm
    for analysis of # of iterations of the loop tries to evaluate.  */
 DEFPARAM(PARAM_MAX_ITERATIONS_TO_TRACK,
Index: tree-ssa-loop-ch.c
===================================================================
--- tree-ssa-loop-ch.c	(revision 237184)
+++ tree-ssa-loop-ch.c	(working copy)
@@ -33,6 +33,7 @@  along with GCC; see the file COPYING3.
 #include "tree-inline.h"
 #include "tree-ssa-scopedtables.h"
 #include "tree-ssa-threadedge.h"
+#include "params.h"
 
 /* Duplicates headers of loops if they are small enough, so that the statements
    in the loop body are always executed when the loop is entered.  This
@@ -106,8 +107,7 @@  should_duplicate_loop_header_p (basic_bl
       return false;
     }
 
-  /* Approximately copy the conditions that used to be used in jump.c --
-     at most 20 insns and no calls.  */
+  /* Count number of instructions and punt on calls.  */
   for (bsi = gsi_start_bb (header); !gsi_end_p (bsi); gsi_next (&bsi))
     {
       last = gsi_stmt (bsi);
@@ -290,8 +290,8 @@  ch_base::copy_headers (function *fun)
 
   FOR_EACH_LOOP (loop, 0)
     {
-      /* Copy at most 20 insns.  */
-      int limit = 20;
+      int ninsns = PARAM_VALUE (PARAM_MAX_LOOP_HEADER_INSNS);
+      int limit = ninsns;
       if (dump_file && (dump_flags & TDF_DETAILS))
 	fprintf (dump_file,
 		 "Analyzing loop %i\n", loop->num);
@@ -333,7 +333,8 @@  ch_base::copy_headers (function *fun)
 	fprintf (dump_file,
 		 "Duplicating header of the loop %d up to edge %d->%d,"
 		 " %i insns.\n",
-		 loop->num, exit->src->index, exit->dest->index, 20 - limit);
+		 loop->num, exit->src->index, exit->dest->index,
+		 ninsns - limit);
 
       /* Ensure that the header will have just the latch as a predecessor
 	 inside the loop.  */