PATCH] PR 49580

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
=
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.
=

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.

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.
>

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.
>>
>