PATCH] PR 49580

PATCH] PR 49580

[Razya Ladelsky]

[-- Attachment #1: Type: text/plain, Size: 880 bytes --]

Hi,

This patch fixes the build failure of gcc spec2006 benchmark.
The change is in  gimple_duplicate_sese_tail(), where we need to subtract 
1 from the loop's number of iterations.
The stmt created to change the rhs of the loop's condition, used to be 
inserted right after the defining stmt of the rhs (an ssa name).
This requires special handling of different cases of the defining stmt:
1.gimple_stmt
2.gimple_phi
3.default_def

Instead of handling each case separately, if we insert the new stmt at the 
begining of the loop's preheader, we're sure that 
it's already been defined.

Bootstrap and testsuite pass successfully (as autopar is not enabled by 
default).
No new regressions when the testsuite is run with autopar enabled.
No new regressions for the run of spec2006 with autopar enabled, and gcc 
benchmark now passes successfully.. 

OK for trunk? 
Thanks,
Razya


[-- Attachment #2: pr49580.txt --]
[-- Type: text/plain, Size: 1042 bytes --]

Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c	(revision 174166)
+++ gcc/tree-cfg.c	(working copy)
@@ -5401,7 +5401,6 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U
   gimple cond_stmt;
   edge sorig, snew;
   basic_block exit_bb;
-  basic_block iters_bb;
   tree new_rhs;
   gimple_stmt_iterator psi;
   gimple phi;
@@ -5501,11 +5500,10 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U
 
     if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME)
       {
-	iters_bb = gimple_bb (SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt)));
-	for (gsi1 = gsi_start_bb (iters_bb); !gsi_end_p (gsi1); gsi_next (&gsi1))
-	  if (gsi_stmt (gsi1) == SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt)))
-	    break;
+	basic_block preheader;
 
+  	preheader = loop_preheader_edge(orig_loop)->src;
+    	gsi1 = gsi_after_labels (preheader);
 	new_rhs = force_gimple_operand_gsi (&gsi1, new_rhs, true,
 					    NULL_TREE,false,GSI_CONTINUE_LINKING);
       }
=

[-- Attachment #3: ChangeLog.txt --]
[-- Type: text/plain, Size: 209 bytes --]


22-12-2009  Razya Ladelsky  <razya@il.ibm.com>

	* tree-cfg.c (gimple_duplicate_sese_tail): Insert the stmt caclculating the new rhs
	 of the loop's condition stmt to the preheader instead of iters_bb.
=

Re: PATCH] PR 49580

[Richard Guenther]

On Thu, Jun 30, 2011 at 2:00 PM, Razya Ladelsky <RAZYA@il.ibm.com> wrote:
> Hi,
>
> This patch fixes the build failure of gcc spec2006 benchmark.
> The change is in  gimple_duplicate_sese_tail(), where we need to subtract
> 1 from the loop's number of iterations.
> The stmt created to change the rhs of the loop's condition, used to be
> inserted right after the defining stmt of the rhs (an ssa name).
> This requires special handling of different cases of the defining stmt:
> 1.gimple_stmt
> 2.gimple_phi
> 3.default_def
>
> Instead of handling each case separately, if we insert the new stmt at the
> begining of the loop's preheader, we're sure that
> it's already been defined.
>
> Bootstrap and testsuite pass successfully (as autopar is not enabled by
> default).
> No new regressions when the testsuite is run with autopar enabled.
> No new regressions for the run of spec2006 with autopar enabled, and gcc
> benchmark now passes successfully..
>
> OK for trunk?

+  	preheader = loop_preheader_edge(orig_loop)->src;

space after loop_preheader_edge.

Otherwise ok.
Thanks,
Richard.

> Thanks,
> Razya
>
>

Re: PATCH] PR 49580

[Zdenek Dvorak]

Hi,

> This patch fixes the build failure of gcc spec2006 benchmark.
> The change is in  gimple_duplicate_sese_tail(), where we need to subtract 
> 1 from the loop's number of iterations.
> The stmt created to change the rhs of the loop's condition, used to be 
> inserted right after the defining stmt of the rhs (an ssa name).
> This requires special handling of different cases of the defining stmt:
> 1.gimple_stmt
> 2.gimple_phi
> 3.default_def
> 
> Instead of handling each case separately, if we insert the new stmt at the 
> begining of the loop's preheader, we're sure that 
> it's already been defined.
> 
> Bootstrap and testsuite pass successfully (as autopar is not enabled by 
> default).
> No new regressions when the testsuite is run with autopar enabled.
> No new regressions for the run of spec2006 with autopar enabled, and gcc 
> benchmark now passes successfully.. 
> 
> OK for trunk? 

actually, I think a better approach would be not to have this kind of pass-specific
adjustments in gimple_duplicate_sese_tail at all.  The code decreasing the number of
iterations in gimple_duplicate_sese_tail only works because parloops does induction
variable canonicalization first; should we need it to be used anywhere else, it will break.
I.e., parloops should first transform the condition so that it does the comparison with
the adjusted value, and then gimple_duplicate_sese_tail could do just code copying
and cfg changes,

Zdenek

Re: PATCH] PR 49580

[Razya Ladelsky]

Zdenek Dvorak <rakdver@kam.mff.cuni.cz> wrote on 30/06/2011 15:21:43:

> From: Zdenek Dvorak <rakdver@kam.mff.cuni.cz>
> To: Razya Ladelsky/Haifa/IBM@IBMIL
> Cc: gcc-patches@gcc.gnu.org, Richard Guenther 
<richard.guenther@gmail.com>
> Date: 30/06/2011 15:21
> Subject: Re: PATCH] PR 49580
> 
> Hi,
> 
> > This patch fixes the build failure of gcc spec2006 benchmark.
> > The change is in  gimple_duplicate_sese_tail(), where we need to 
subtract 
> > 1 from the loop's number of iterations.
> > The stmt created to change the rhs of the loop's condition, used to be 

> > inserted right after the defining stmt of the rhs (an ssa name).
> > This requires special handling of different cases of the defining 
stmt:
> > 1.gimple_stmt
> > 2.gimple_phi
> > 3.default_def
> > 
> > Instead of handling each case separately, if we insert the new stmt at 
the 
> > begining of the loop's preheader, we're sure that 
> > it's already been defined.
> > 
> > Bootstrap and testsuite pass successfully (as autopar is not enabled 
by 
> > default).
> > No new regressions when the testsuite is run with autopar enabled.
> > No new regressions for the run of spec2006 with autopar enabled, and 
gcc 
> > benchmark now passes successfully.. 
> > 
> > OK for trunk? 
> 
> actually, I think a better approach would be not to have this kind 
> of pass-specific
> adjustments in gimple_duplicate_sese_tail at all.  The code 
> decreasing the number of
> iterations in gimple_duplicate_sese_tail only works because parloops
> does induction
> variable canonicalization first; should we need it to be used 
> anywhere else, it will break.
> I.e., parloops should first transform the condition so that it does 
> the comparison with
> the adjusted value, and then gimple_duplicate_sese_tail could do 
> just code copying
> and cfg changes,
> 

Ok, working on it.
Thanks,
Razya

> Zdenek

Re: PATCH] PR 49580

[Razya Ladelsky]

[-- Attachment #1: Type: text/plain, Size: 2097 bytes --]

Zdenek Dvorak <rakdver@kam.mff.cuni.cz> wrote on 30/06/2011 15:21:43:

> From: Zdenek Dvorak <rakdver@kam.mff.cuni.cz>
> To: Razya Ladelsky/Haifa/IBM@IBMIL
> Cc: gcc-patches@gcc.gnu.org, Richard Guenther 
<richard.guenther@gmail.com>
> Date: 30/06/2011 15:21
> Subject: Re: PATCH] PR 49580
> 
> Hi,
> 
> > This patch fixes the build failure of gcc spec2006 benchmark.
> > The change is in  gimple_duplicate_sese_tail(), where we need to 
subtract 
> > 1 from the loop's number of iterations.
> > The stmt created to change the rhs of the loop's condition, used to be 

> > inserted right after the defining stmt of the rhs (an ssa name).
> > This requires special handling of different cases of the defining 
stmt:
> > 1.gimple_stmt
> > 2.gimple_phi
> > 3.default_def
> > 
> > Instead of handling each case separately, if we insert the new stmt at 
the 
> > begining of the loop's preheader, we're sure that 
> > it's already been defined.
> > 
> > Bootstrap and testsuite pass successfully (as autopar is not enabled 
by 
> > default).
> > No new regressions when the testsuite is run with autopar enabled.
> > No new regressions for the run of spec2006 with autopar enabled, and 
gcc 
> > benchmark now passes successfully.. 
> > 
> > OK for trunk? 
> 
> actually, I think a better approach would be not to have this kind 
> of pass-specific
> adjustments in gimple_duplicate_sese_tail at all.  The code 
> decreasing the number of
> iterations in gimple_duplicate_sese_tail only works because parloops
> does induction
> variable canonicalization first; should we need it to be used 
> anywhere else, it will break.
> I.e., parloops should first transform the condition so that it does 
> the comparison with
> the adjusted value, and then gimple_duplicate_sese_tail could do 
> just code copying
> and cfg changes,
> 
> Zdenek

Hi,

I moved the adjustment of the loop's iterations from 
gimple_duplicate_sese_tail
to tree-parloops.c, right before the call to gimple_duplicate_sese_tail.
I repeated the bootstrap, regression and spec runs - no new regressions.

OK to commit?
Thanks,
razya





[-- Attachment #2: gcc_patch_new.txt --]
[-- Type: text/plain, Size: 4338 bytes --]

Index: gcc/tree-parloops.c
===================================================================
--- gcc/tree-parloops.c	(revision 174166)
+++ gcc/tree-parloops.c	(working copy)
@@ -1464,6 +1464,8 @@ transform_to_exit_first_loop (struct loop *loop, h
   gimple phi, nphi, cond_stmt, stmt, cond_nit;
   gimple_stmt_iterator gsi;
   tree nit_1;
+  edge exit_1;
+  tree new_rhs;
 
   split_block_after_labels (loop->header);
   orig_header = single_succ (loop->header);
@@ -1492,6 +1494,38 @@ transform_to_exit_first_loop (struct loop *loop, h
 	  control = t;
 	}
     }
+
+ /* Setting the condition towards peeling the last iteration:
+    If the block consisting of the exit condition has the latch as
+    successor, then the body of the loop is executed before
+    the exit condition is tested.  In such case, moving the
+    condition to the entry, causes that the loop will iterate
+    one less iteration (which is the wanted outcome, since we
+    peel out the last iteration).  If the body is executed after
+    the condition, moving the condition to the entry requires
+    decrementing one iteration.  */
+  exit_1 = EDGE_SUCC (exit->src, EDGE_SUCC (exit->src, 0) == exit); 
+  if (exit_1->dest == loop->latch)
+    new_rhs = gimple_cond_rhs (cond_stmt);
+  else
+  {
+    new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs (cond_stmt)),
+			   gimple_cond_rhs (cond_stmt),
+			   build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 1));
+    if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME)
+      {
+ 	basic_block preheader;
+  	gimple_stmt_iterator gsi1;
+
+  	preheader = loop_preheader_edge(loop)->src;
+    	gsi1 = gsi_after_labels (preheader);
+	new_rhs = force_gimple_operand_gsi (&gsi1, new_rhs, true,
+					    NULL_TREE,false,GSI_CONTINUE_LINKING);
+      }
+  }
+  gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs));
+  gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs (cond_stmt)));
+  
   bbs = get_loop_body_in_dom_order (loop);
 
   for (n = 0; bbs[n] != loop->latch; n++)
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c	(revision 174166)
+++ gcc/tree-cfg.c	(working copy)
@@ -5397,12 +5397,10 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U
   int total_freq = 0, exit_freq = 0;
   gcov_type total_count = 0, exit_count = 0;
   edge exits[2], nexits[2], e;
-  gimple_stmt_iterator gsi,gsi1;
+  gimple_stmt_iterator gsi;
   gimple cond_stmt;
   edge sorig, snew;
   basic_block exit_bb;
-  basic_block iters_bb;
-  tree new_rhs;
   gimple_stmt_iterator psi;
   gimple phi;
   tree def;
@@ -5483,35 +5481,6 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U
   gcc_assert (gimple_code (cond_stmt) == GIMPLE_COND);
   cond_stmt = gimple_copy (cond_stmt);
 
- /* If the block consisting of the exit condition has the latch as
-    successor, then the body of the loop is executed before
-    the exit condition is tested.  In such case, moving the
-    condition to the entry, causes that the loop will iterate
-    one less iteration (which is the wanted outcome, since we
-    peel out the last iteration).  If the body is executed after
-    the condition, moving the condition to the entry requires
-    decrementing one iteration.  */
-  if (exits[1]->dest == orig_loop->latch)
-    new_rhs = gimple_cond_rhs (cond_stmt);
-  else
-  {
-    new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs (cond_stmt)),
-			   gimple_cond_rhs (cond_stmt),
-			   build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 1));
-
-    if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME)
-      {
-	iters_bb = gimple_bb (SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt)));
-	for (gsi1 = gsi_start_bb (iters_bb); !gsi_end_p (gsi1); gsi_next (&gsi1))
-	  if (gsi_stmt (gsi1) == SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt)))
-	    break;
-
-	new_rhs = force_gimple_operand_gsi (&gsi1, new_rhs, true,
-					    NULL_TREE,false,GSI_CONTINUE_LINKING);
-      }
-  }
-  gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs));
-  gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs (cond_stmt)));
   gsi_insert_after (&gsi, cond_stmt, GSI_NEW_STMT);
 
   sorig = single_succ_edge (switch_bb);
=

[-- Attachment #3: ChangeLog.txt --]
[-- Type: text/plain, Size: 436 bytes --]


07-05-2011  Razya Ladelsky  <razya@il.ibm.com>

	* tree-cfg.c (gimple_duplicate_sese_tail): Remove handling of 
	the loop's number of iterations.
        * tree-parloops.c (transform_to_exit_first_loop): Add the 
	handling of the loop's number of iterations before the call 
	to gimple_duplicate_sese_tail.
        Insert the stmt caclculating the new rhs of the loop's
	condition stmt to the preheader instead of iters_bb.
=

Re: PATCH] PR 49580

[Zdenek Dvorak]

Hi,

> I moved the adjustment of the loop's iterations from 
> gimple_duplicate_sese_tail
> to tree-parloops.c, right before the call to gimple_duplicate_sese_tail.
> I repeated the bootstrap, regression and spec runs - no new regressions.
> 
> OK to commit?

OK,

Zdenek

> Index: gcc/tree-parloops.c
> ===================================================================
> --- gcc/tree-parloops.c	(revision 174166)
> +++ gcc/tree-parloops.c	(working copy)
> @@ -1464,6 +1464,8 @@ transform_to_exit_first_loop (struct loop *loop, h
>    gimple phi, nphi, cond_stmt, stmt, cond_nit;
>    gimple_stmt_iterator gsi;
>    tree nit_1;
> +  edge exit_1;
> +  tree new_rhs;
>  
>    split_block_after_labels (loop->header);
>    orig_header = single_succ (loop->header);
> @@ -1492,6 +1494,38 @@ transform_to_exit_first_loop (struct loop *loop, h
>  	  control = t;
>  	}
>      }
> +
> + /* Setting the condition towards peeling the last iteration:
> +    If the block consisting of the exit condition has the latch as
> +    successor, then the body of the loop is executed before
> +    the exit condition is tested.  In such case, moving the
> +    condition to the entry, causes that the loop will iterate
> +    one less iteration (which is the wanted outcome, since we
> +    peel out the last iteration).  If the body is executed after
> +    the condition, moving the condition to the entry requires
> +    decrementing one iteration.  */
> +  exit_1 = EDGE_SUCC (exit->src, EDGE_SUCC (exit->src, 0) == exit); 
> +  if (exit_1->dest == loop->latch)
> +    new_rhs = gimple_cond_rhs (cond_stmt);
> +  else
> +  {
> +    new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs (cond_stmt)),
> +			   gimple_cond_rhs (cond_stmt),
> +			   build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 1));
> +    if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME)
> +      {
> + 	basic_block preheader;
> +  	gimple_stmt_iterator gsi1;
> +
> +  	preheader = loop_preheader_edge(loop)->src;
> +    	gsi1 = gsi_after_labels (preheader);
> +	new_rhs = force_gimple_operand_gsi (&gsi1, new_rhs, true,
> +					    NULL_TREE,false,GSI_CONTINUE_LINKING);
> +      }
> +  }
> +  gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs));
> +  gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs (cond_stmt)));
> +  
>    bbs = get_loop_body_in_dom_order (loop);
>  
>    for (n = 0; bbs[n] != loop->latch; n++)
> Index: gcc/tree-cfg.c
> ===================================================================
> --- gcc/tree-cfg.c	(revision 174166)
> +++ gcc/tree-cfg.c	(working copy)
> @@ -5397,12 +5397,10 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U
>    int total_freq = 0, exit_freq = 0;
>    gcov_type total_count = 0, exit_count = 0;
>    edge exits[2], nexits[2], e;
> -  gimple_stmt_iterator gsi,gsi1;
> +  gimple_stmt_iterator gsi;
>    gimple cond_stmt;
>    edge sorig, snew;
>    basic_block exit_bb;
> -  basic_block iters_bb;
> -  tree new_rhs;
>    gimple_stmt_iterator psi;
>    gimple phi;
>    tree def;
> @@ -5483,35 +5481,6 @@ gimple_duplicate_sese_tail (edge entry ATTRIBUTE_U
>    gcc_assert (gimple_code (cond_stmt) == GIMPLE_COND);
>    cond_stmt = gimple_copy (cond_stmt);
>  
> - /* If the block consisting of the exit condition has the latch as
> -    successor, then the body of the loop is executed before
> -    the exit condition is tested.  In such case, moving the
> -    condition to the entry, causes that the loop will iterate
> -    one less iteration (which is the wanted outcome, since we
> -    peel out the last iteration).  If the body is executed after
> -    the condition, moving the condition to the entry requires
> -    decrementing one iteration.  */
> -  if (exits[1]->dest == orig_loop->latch)
> -    new_rhs = gimple_cond_rhs (cond_stmt);
> -  else
> -  {
> -    new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs (cond_stmt)),
> -			   gimple_cond_rhs (cond_stmt),
> -			   build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 1));
> -
> -    if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME)
> -      {
> -	iters_bb = gimple_bb (SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt)));
> -	for (gsi1 = gsi_start_bb (iters_bb); !gsi_end_p (gsi1); gsi_next (&gsi1))
> -	  if (gsi_stmt (gsi1) == SSA_NAME_DEF_STMT (gimple_cond_rhs (cond_stmt)))
> -	    break;
> -
> -	new_rhs = force_gimple_operand_gsi (&gsi1, new_rhs, true,
> -					    NULL_TREE,false,GSI_CONTINUE_LINKING);
> -      }
> -  }
> -  gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs));
> -  gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs (cond_stmt)));
>    gsi_insert_after (&gsi, cond_stmt, GSI_NEW_STMT);
>  
>    sorig = single_succ_edge (switch_bb);

> 
> 07-05-2011  Razya Ladelsky  <razya@il.ibm.com>
> 
> 	* tree-cfg.c (gimple_duplicate_sese_tail): Remove handling of 
> 	the loop's number of iterations.
>         * tree-parloops.c (transform_to_exit_first_loop): Add the 
> 	handling of the loop's number of iterations before the call 
> 	to gimple_duplicate_sese_tail.
>         Insert the stmt caclculating the new rhs of the loop's
> 	condition stmt to the preheader instead of iters_bb.

Re: PATCH] PR 49580

[Razya Ladelsky]

[-- Attachment #1: Type: text/plain, Size: 5991 bytes --]

Zdenek Dvorak <rakdver@kam.mff.cuni.cz> wrote on 05/07/2011 13:37:41:

> From: Zdenek Dvorak <rakdver@kam.mff.cuni.cz>
> To: Razya Ladelsky/Haifa/IBM@IBMIL
> Cc: gcc-patches@gcc.gnu.org, Richard Guenther 
<richard.guenther@gmail.com>
> Date: 05/07/2011 13:37
> Subject: Re: PATCH] PR 49580
> 
> Hi,
> 
> > I moved the adjustment of the loop's iterations from 
> > gimple_duplicate_sese_tail
> > to tree-parloops.c, right before the call to 
gimple_duplicate_sese_tail.
> > I repeated the bootstrap, regression and spec runs - no new 
regressions.
> > 
> > OK to commit?
> 
> OK,
> 
> Zdenek

I also want to commit this testcase, which is a reduced case of the gcc 
benchmark.
I apologize for not submitting it together with the patch.
OK?

Thanks,
Razya




> 
> > Index: gcc/tree-parloops.c
> > ===================================================================
> > --- gcc/tree-parloops.c   (revision 174166)
> > +++ gcc/tree-parloops.c   (working copy)
> > @@ -1464,6 +1464,8 @@ transform_to_exit_first_loop (struct loop *loop, 
h
> >    gimple phi, nphi, cond_stmt, stmt, cond_nit;
> >    gimple_stmt_iterator gsi;
> >    tree nit_1;
> > +  edge exit_1;
> > +  tree new_rhs;
> > 
> >    split_block_after_labels (loop->header);
> >    orig_header = single_succ (loop->header);
> > @@ -1492,6 +1494,38 @@ transform_to_exit_first_loop (struct loop 
*loop, h
> >       control = t;
> >     }
> >      }
> > +
> > + /* Setting the condition towards peeling the last iteration:
> > +    If the block consisting of the exit condition has the latch as
> > +    successor, then the body of the loop is executed before
> > +    the exit condition is tested.  In such case, moving the
> > +    condition to the entry, causes that the loop will iterate
> > +    one less iteration (which is the wanted outcome, since we
> > +    peel out the last iteration).  If the body is executed after
> > +    the condition, moving the condition to the entry requires
> > +    decrementing one iteration.  */
> > +  exit_1 = EDGE_SUCC (exit->src, EDGE_SUCC (exit->src, 0) == exit); 
> > +  if (exit_1->dest == loop->latch)
> > +    new_rhs = gimple_cond_rhs (cond_stmt);
> > +  else
> > +  {
> > +    new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs
> (cond_stmt)),
> > +            gimple_cond_rhs (cond_stmt),
> > +            build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 
1));
> > +    if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME)
> > +      {
> > +    basic_block preheader;
> > +     gimple_stmt_iterator gsi1;
> > +
> > +     preheader = loop_preheader_edge(loop)->src;
> > +       gsi1 = gsi_after_labels (preheader);
> > +   new_rhs = force_gimple_operand_gsi (&gsi1, new_rhs, true,
> > +                   NULL_TREE,false,GSI_CONTINUE_LINKING);
> > +      }
> > +  }
> > +  gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs));
> > +  gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs 
> (cond_stmt)));
> > + 
> >    bbs = get_loop_body_in_dom_order (loop);
> > 
> >    for (n = 0; bbs[n] != loop->latch; n++)
> > Index: gcc/tree-cfg.c
> > ===================================================================
> > --- gcc/tree-cfg.c   (revision 174166)
> > +++ gcc/tree-cfg.c   (working copy)
> > @@ -5397,12 +5397,10 @@ gimple_duplicate_sese_tail (edge entry 
ATTRIBUTE_U
> >    int total_freq = 0, exit_freq = 0;
> >    gcov_type total_count = 0, exit_count = 0;
> >    edge exits[2], nexits[2], e;
> > -  gimple_stmt_iterator gsi,gsi1;
> > +  gimple_stmt_iterator gsi;
> >    gimple cond_stmt;
> >    edge sorig, snew;
> >    basic_block exit_bb;
> > -  basic_block iters_bb;
> > -  tree new_rhs;
> >    gimple_stmt_iterator psi;
> >    gimple phi;
> >    tree def;
> > @@ -5483,35 +5481,6 @@ gimple_duplicate_sese_tail (edge entry 
ATTRIBUTE_U
> >    gcc_assert (gimple_code (cond_stmt) == GIMPLE_COND);
> >    cond_stmt = gimple_copy (cond_stmt);
> > 
> > - /* If the block consisting of the exit condition has the latch as
> > -    successor, then the body of the loop is executed before
> > -    the exit condition is tested.  In such case, moving the
> > -    condition to the entry, causes that the loop will iterate
> > -    one less iteration (which is the wanted outcome, since we
> > -    peel out the last iteration).  If the body is executed after
> > -    the condition, moving the condition to the entry requires
> > -    decrementing one iteration.  */
> > -  if (exits[1]->dest == orig_loop->latch)
> > -    new_rhs = gimple_cond_rhs (cond_stmt);
> > -  else
> > -  {
> > -    new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs
> (cond_stmt)),
> > -            gimple_cond_rhs (cond_stmt),
> > -            build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)), 
1));
> > -
> > -    if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME)
> > -      {
> > -   iters_bb = gimple_bb (SSA_NAME_DEF_STMT (gimple_cond_rhs 
(cond_stmt)));
> > -   for (gsi1 = gsi_start_bb (iters_bb); !gsi_end_p (gsi1); 
> gsi_next (&gsi1))
> > -     if (gsi_stmt (gsi1) == SSA_NAME_DEF_STMT (gimple_cond_rhs 
> (cond_stmt)))
> > -       break;
> > -
> > -   new_rhs = force_gimple_operand_gsi (&gsi1, new_rhs, true,
> > -                   NULL_TREE,false,GSI_CONTINUE_LINKING);
> > -      }
> > -  }
> > -  gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs));
> > -  gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs 
> (cond_stmt)));
> >    gsi_insert_after (&gsi, cond_stmt, GSI_NEW_STMT);
> > 
> >    sorig = single_succ_edge (switch_bb);
> 
> > 
> > 07-05-2011  Razya Ladelsky  <razya@il.ibm.com>
> > 
> >    * tree-cfg.c (gimple_duplicate_sese_tail): Remove handling of 
> >    the loop's number of iterations.
> >         * tree-parloops.c (transform_to_exit_first_loop): Add the 
> >    handling of the loop's number of iterations before the call 
> >    to gimple_duplicate_sese_tail.
> >         Insert the stmt caclculating the new rhs of the loop's
> >    condition stmt to the preheader instead of iters_bb.
> 

[-- Attachment #2: gcc_test.c --]
[-- Type: application/octet-stream, Size: 886 bytes --]

/* { dg-do compile } */
/* { dg-options "-O2 -ftree-parallelize-loops=4 -fdump-tree-parloops-details -fdump-tree-optimized" } */

#include <stdarg.h>
#include <stdlib.h>

#define N 1600

unsigned int ub[N];
unsigned char reg_has_output_reload[N];
unsigned int uc[N];

 __attribute__ ((noinline)) 
 void main2 (unsigned int regno, unsigned int n_reloads)
 {
  unsigned int nr = 0;

  if (regno > ub[regno])
    nr = regno;
  else
    nr = ub[nr];

  while (nr-- > 0)
    if (n_reloads == 0 || reg_has_output_reload[regno + nr] == 0)
      ub[regno + nr] = 0;
}

int main (void)
{ 
  main2 (799, 0);
  return 0;
}


/* { dg-final { scan-tree-dump-times "Detected reduction" 3 "parloops" } } */
/* { dg-final { scan-tree-dump-times "SUCCESS: may be parallelized" 4 "parloops" } } */
/* { dg-final { cleanup-tree-dump "parloops" } } */
/* { dg-final { cleanup-tree-dump "optimized" } } */

Re: PATCH] PR 49580

[Richard Guenther]

On Tue, Jul 5, 2011 at 1:08 PM, Razya Ladelsky <RAZYA@il.ibm.com> wrote:
> Zdenek Dvorak <rakdver@kam.mff.cuni.cz> wrote on 05/07/2011 13:37:41:
>
>> From: Zdenek Dvorak <rakdver@kam.mff.cuni.cz>
>> To: Razya Ladelsky/Haifa/IBM@IBMIL
>> Cc: gcc-patches@gcc.gnu.org, Richard Guenther
> <richard.guenther@gmail.com>
>> Date: 05/07/2011 13:37
>> Subject: Re: PATCH] PR 49580
>>
>> Hi,
>>
>> > I moved the adjustment of the loop's iterations from
>> > gimple_duplicate_sese_tail
>> > to tree-parloops.c, right before the call to
> gimple_duplicate_sese_tail.
>> > I repeated the bootstrap, regression and spec runs - no new
> regressions.
>> >
>> > OK to commit?
>>
>> OK,
>>
>> Zdenek
>
> I also want to commit this testcase, which is a reduced case of the gcc
> benchmark.
> I apologize for not submitting it together with the patch.
> OK?

Ok.

Thanks,
Richard.

> Thanks,
> Razya
>
>
>
>
>>
>> > Index: gcc/tree-parloops.c
>> > ===================================================================
>> > --- gcc/tree-parloops.c   (revision 174166)
>> > +++ gcc/tree-parloops.c   (working copy)
>> > @@ -1464,6 +1464,8 @@ transform_to_exit_first_loop (struct loop *loop,
> h
>> >    gimple phi, nphi, cond_stmt, stmt, cond_nit;
>> >    gimple_stmt_iterator gsi;
>> >    tree nit_1;
>> > +  edge exit_1;
>> > +  tree new_rhs;
>> >
>> >    split_block_after_labels (loop->header);
>> >    orig_header = single_succ (loop->header);
>> > @@ -1492,6 +1494,38 @@ transform_to_exit_first_loop (struct loop
> *loop, h
>> >       control = t;
>> >     }
>> >      }
>> > +
>> > + /* Setting the condition towards peeling the last iteration:
>> > +    If the block consisting of the exit condition has the latch as
>> > +    successor, then the body of the loop is executed before
>> > +    the exit condition is tested.  In such case, moving the
>> > +    condition to the entry, causes that the loop will iterate
>> > +    one less iteration (which is the wanted outcome, since we
>> > +    peel out the last iteration).  If the body is executed after
>> > +    the condition, moving the condition to the entry requires
>> > +    decrementing one iteration.  */
>> > +  exit_1 = EDGE_SUCC (exit->src, EDGE_SUCC (exit->src, 0) == exit);
>> > +  if (exit_1->dest == loop->latch)
>> > +    new_rhs = gimple_cond_rhs (cond_stmt);
>> > +  else
>> > +  {
>> > +    new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs
>> (cond_stmt)),
>> > +            gimple_cond_rhs (cond_stmt),
>> > +            build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)),
> 1));
>> > +    if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME)
>> > +      {
>> > +    basic_block preheader;
>> > +     gimple_stmt_iterator gsi1;
>> > +
>> > +     preheader = loop_preheader_edge(loop)->src;
>> > +       gsi1 = gsi_after_labels (preheader);
>> > +   new_rhs = force_gimple_operand_gsi (&gsi1, new_rhs, true,
>> > +                   NULL_TREE,false,GSI_CONTINUE_LINKING);
>> > +      }
>> > +  }
>> > +  gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs));
>> > +  gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs
>> (cond_stmt)));
>> > +
>> >    bbs = get_loop_body_in_dom_order (loop);
>> >
>> >    for (n = 0; bbs[n] != loop->latch; n++)
>> > Index: gcc/tree-cfg.c
>> > ===================================================================
>> > --- gcc/tree-cfg.c   (revision 174166)
>> > +++ gcc/tree-cfg.c   (working copy)
>> > @@ -5397,12 +5397,10 @@ gimple_duplicate_sese_tail (edge entry
> ATTRIBUTE_U
>> >    int total_freq = 0, exit_freq = 0;
>> >    gcov_type total_count = 0, exit_count = 0;
>> >    edge exits[2], nexits[2], e;
>> > -  gimple_stmt_iterator gsi,gsi1;
>> > +  gimple_stmt_iterator gsi;
>> >    gimple cond_stmt;
>> >    edge sorig, snew;
>> >    basic_block exit_bb;
>> > -  basic_block iters_bb;
>> > -  tree new_rhs;
>> >    gimple_stmt_iterator psi;
>> >    gimple phi;
>> >    tree def;
>> > @@ -5483,35 +5481,6 @@ gimple_duplicate_sese_tail (edge entry
> ATTRIBUTE_U
>> >    gcc_assert (gimple_code (cond_stmt) == GIMPLE_COND);
>> >    cond_stmt = gimple_copy (cond_stmt);
>> >
>> > - /* If the block consisting of the exit condition has the latch as
>> > -    successor, then the body of the loop is executed before
>> > -    the exit condition is tested.  In such case, moving the
>> > -    condition to the entry, causes that the loop will iterate
>> > -    one less iteration (which is the wanted outcome, since we
>> > -    peel out the last iteration).  If the body is executed after
>> > -    the condition, moving the condition to the entry requires
>> > -    decrementing one iteration.  */
>> > -  if (exits[1]->dest == orig_loop->latch)
>> > -    new_rhs = gimple_cond_rhs (cond_stmt);
>> > -  else
>> > -  {
>> > -    new_rhs = fold_build2 (MINUS_EXPR, TREE_TYPE (gimple_cond_rhs
>> (cond_stmt)),
>> > -            gimple_cond_rhs (cond_stmt),
>> > -            build_int_cst (TREE_TYPE (gimple_cond_rhs (cond_stmt)),
> 1));
>> > -
>> > -    if (TREE_CODE (gimple_cond_rhs (cond_stmt)) == SSA_NAME)
>> > -      {
>> > -   iters_bb = gimple_bb (SSA_NAME_DEF_STMT (gimple_cond_rhs
> (cond_stmt)));
>> > -   for (gsi1 = gsi_start_bb (iters_bb); !gsi_end_p (gsi1);
>> gsi_next (&gsi1))
>> > -     if (gsi_stmt (gsi1) == SSA_NAME_DEF_STMT (gimple_cond_rhs
>> (cond_stmt)))
>> > -       break;
>> > -
>> > -   new_rhs = force_gimple_operand_gsi (&gsi1, new_rhs, true,
>> > -                   NULL_TREE,false,GSI_CONTINUE_LINKING);
>> > -      }
>> > -  }
>> > -  gimple_cond_set_rhs (cond_stmt, unshare_expr (new_rhs));
>> > -  gimple_cond_set_lhs (cond_stmt, unshare_expr (gimple_cond_lhs
>> (cond_stmt)));
>> >    gsi_insert_after (&gsi, cond_stmt, GSI_NEW_STMT);
>> >
>> >    sorig = single_succ_edge (switch_bb);
>>
>> >
>> > 07-05-2011  Razya Ladelsky  <razya@il.ibm.com>
>> >
>> >    * tree-cfg.c (gimple_duplicate_sese_tail): Remove handling of
>> >    the loop's number of iterations.
>> >         * tree-parloops.c (transform_to_exit_first_loop): Add the
>> >    handling of the loop's number of iterations before the call
>> >    to gimple_duplicate_sese_tail.
>> >         Insert the stmt caclculating the new rhs of the loop's
>> >    condition stmt to the preheader instead of iters_bb.
>>
>