Richard Guenther <rguent...@suse.de> wrote on 26/03/2012 01:23:15 PM:
> From: Richard Guenther <rguent...@suse.de>
> To: Razya Ladelsky/Haifa/IBM@IBMIL
> Cc: gcc-patches@gcc.gnu.org
> Date: 26/03/2012 01:23 PM
> Subject: Re: [PATCH] Permanent Fix for PR46886
>
> On Mon, 26 Mar 2012, Razya Ladelsky wrote:
>
> > Hi,
> >
> > This is (hopefully) a permanent fix to pr46886.c
> > I removed the condition preventing parallelization of do_while loops,
as
> > it
> > was blocking parallelizing important loops in spec-2006.
> > The patch fixes the number of iterations for cases where the body
could
> > appear in the latch, as in pr46886.c.
> >
> > 2012-03-26 Razya Ladelsky <ra...@il.ibm.com>
> >
> > PR tree-optimization/46886
> > * tree-parloops.c (transform_to_exit_first_loop):Set
> > number of iterations correctly when the body may appear at the latch.
> > (pallelize_loops): Remove the condition preventing
> > do-while loops.
> >
> >
> >
> > Bootstrap and testsuite psss successfully on power7 linux machine.
> > Ok to commit?
>
> +
> + /* if the latch contains more than the one statemnt of control
variable
> + increment then it contains the body. */
> + if (exit_1->dest == loop->latch && last_and_only_stmt (loop->latch))
> new_rhs = gimple_cond_rhs (cond_stmt);
>
> please check what the comment suggests, thus,
>
> && last_and_only_stmt (loop->latch) == cond_stmt
>
Hi Richard,
The latch has at least one stmt for sure:
the control variable increment (not a cond_stmt) coming from the call to
canonicalize_loop_ivs(loop, nit, true) earlier in tree-parloops:
/* ..... When BUMP_IN_LATCH is true, the induction
variable is incremented in the loop latch, otherwise it is
incremented in the loop header. Return the induction variable that
was created. */
tree
canonicalize_loop_ivs (struct loop *loop, tree *nit, bool bump_in_latch)
> tree-parloops.c is quite fragile in what it expects the IL to look like
> and tests do not cover what later code expects. Please do not add to
> that.
I agree.
I have also tested this code on spec2006 benchmarks.
(together with an upcoming patch, 5 of these show speedups with autopar on
a linux-power7)
>
> @@ -2146,7 +2149,6 @@ parallelize_loops (void)
> return false;
> if (cfun->has_nonlocal_label)
> return false;
> -
>
> spurious whitespace change.
>
> @@ -2213,6 +2212,7 @@ parallelize_loops (void)
> continue;
>
> changed = true;
> +
> if (dump_file && (dump_flags & TDF_DETAILS))
> {
> if (loop->inner)
>
> Likewise.
>
> Ok with the above change.
Ok, thanks,
Razya
>
> Thanks,
> Richard.
>
