Message ID | 557571B5.1060903@mentor.com |
---|---|
State | New |
Headers | show |
On Mon, 8 Jun 2015, Tom de Vries wrote: > On 04/06/15 10:28, Tom de Vries wrote: > > > I'm ok with the patch and count on you to fix eventual fallout ;) > > > > > > > Great, will do. > > And here is the fallout: > * PR66442 - [6 regression] FAIL: gcc.dg/autopar/pr46885.c (test for > excess errors) > > There are two problems in try_transform_to_exit_first_loop_alt: > 1. In case the latch is not a singleton bb, the function should return > false rather than true. > 2. The check for singleton bb should ignore debug-insns. > > Attached patch fixes these problems. > > Bootstrapped and reg-tested on x86_64. > > Verified by Andreas to fix the problem on m68k. > > OK for trunk? Ok. Thanks, Richard.
Hi Tom! On Mon, 8 Jun 2015 12:43:01 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote: > There are two problems in try_transform_to_exit_first_loop_alt: > 1. In case the latch is not a singleton bb, the function should return > false rather than true. > 2. The check for singleton bb should ignore debug-insns. > > Attached patch fixes these problems. > Fix try_transform_to_exit_first_loop_alt > PR tree-optimization/66442 > * gimple-iterator.h (gimple_seq_nondebug_singleton_p): Add function. > * tree-parloops.c (try_transform_to_exit_first_loop_alt): Return false > if the loop latch is not a singleton. Use > gimple_seq_nondebug_singleton_p instead of gimple_seq_singleton_p. Per my testing, the backport of this patch that you committed to gomp-4_0-branch, r224219, introduces a number of regressions in your OpenACC kernels test cases, specifically the »scan-tree-dump-times parloops_oacc_kernels "(?n)pragma omp target oacc_parallel.*num_gangs\\(32\\)" 1« tests. Would you please have a look? Grüße, Thomas > gcc/gimple-iterator.h | 29 +++++++++++++++++++++++++++++ > gcc/tree-parloops.c | 4 ++-- > 2 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h > index 87e943a..76fa456 100644 > --- a/gcc/gimple-iterator.h > +++ b/gcc/gimple-iterator.h > @@ -345,4 +345,33 @@ gsi_seq (gimple_stmt_iterator i) > return *i.seq; > } > > +/* Determine whether SEQ is a nondebug singleton. */ > + > +static inline bool > +gimple_seq_nondebug_singleton_p (gimple_seq seq) > +{ > + gimple_stmt_iterator gsi; > + > + /* Find a nondebug gimple. */ > + gsi.ptr = gimple_seq_first (seq); > + gsi.seq = &seq; > + gsi.bb = NULL; > + while (!gsi_end_p (gsi) > + && is_gimple_debug (gsi_stmt (gsi))) > + gsi_next (&gsi); > + > + /* No nondebug gimple found, not a singleton. */ > + if (gsi_end_p (gsi)) > + return false; > + > + /* Find a next nondebug gimple. */ > + gsi_next (&gsi); > + while (!gsi_end_p (gsi) > + && is_gimple_debug (gsi_stmt (gsi))) > + gsi_next (&gsi); > + > + /* Only a singleton if there's no next nondebug gimple. */ > + return gsi_end_p (gsi); > +} > + > #endif /* GCC_GIMPLE_ITERATOR_H */ > diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c > index 02f44eb..c4b83fe 100644 > --- a/gcc/tree-parloops.c > +++ b/gcc/tree-parloops.c > @@ -1769,8 +1769,8 @@ try_transform_to_exit_first_loop_alt (struct loop *loop, > tree nit) > { > /* Check whether the latch contains a single statement. */ > - if (!gimple_seq_singleton_p (bb_seq (loop->latch))) > - return true; > + if (!gimple_seq_nondebug_singleton_p (bb_seq (loop->latch))) > + return false; > > /* Check whether the latch contains the loop iv increment. */ > edge back = single_succ_edge (loop->latch); > -- > 1.9.1 >
On 08/06/15 17:55, Thomas Schwinge wrote: > Hi Tom! > > On Mon, 8 Jun 2015 12:43:01 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote: >> There are two problems in try_transform_to_exit_first_loop_alt: >> 1. In case the latch is not a singleton bb, the function should return >> false rather than true. >> 2. The check for singleton bb should ignore debug-insns. >> >> Attached patch fixes these problems. > >> Fix try_transform_to_exit_first_loop_alt > >> PR tree-optimization/66442 >> * gimple-iterator.h (gimple_seq_nondebug_singleton_p): Add function. >> * tree-parloops.c (try_transform_to_exit_first_loop_alt): Return false >> if the loop latch is not a singleton. Use >> gimple_seq_nondebug_singleton_p instead of gimple_seq_singleton_p. > > Per my testing, the backport of this patch that you committed to > gomp-4_0-branch, r224219, introduces a number of regressions in your > OpenACC kernels test cases, specifically the »scan-tree-dump-times > parloops_oacc_kernels "(?n)pragma omp target > oacc_parallel.*num_gangs\\(32\\)" 1« tests. Would you please have a > look? > > Hi Thomas, I seem to have committed (to both trunk and gomp-4_0-branch) an older version of the patch, which contained an incorrect version of gimple_seq_nondebug_singleton_p. I'll correct the mistake tomorrow morning. Thanks, - Tom > Grüße, > Thomas > > >> gcc/gimple-iterator.h | 29 +++++++++++++++++++++++++++++ >> gcc/tree-parloops.c | 4 ++-- >> 2 files changed, 31 insertions(+), 2 deletions(-) >> >> diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h >> index 87e943a..76fa456 100644 >> --- a/gcc/gimple-iterator.h >> +++ b/gcc/gimple-iterator.h >> @@ -345,4 +345,33 @@ gsi_seq (gimple_stmt_iterator i) >> return *i.seq; >> } >> >> +/* Determine whether SEQ is a nondebug singleton. */ >> + >> +static inline bool >> +gimple_seq_nondebug_singleton_p (gimple_seq seq) >> +{ >> + gimple_stmt_iterator gsi; >> + >> + /* Find a nondebug gimple. */ >> + gsi.ptr = gimple_seq_first (seq); >> + gsi.seq = &seq; >> + gsi.bb = NULL; >> + while (!gsi_end_p (gsi) >> + && is_gimple_debug (gsi_stmt (gsi))) >> + gsi_next (&gsi); >> + >> + /* No nondebug gimple found, not a singleton. */ >> + if (gsi_end_p (gsi)) >> + return false; >> + >> + /* Find a next nondebug gimple. */ >> + gsi_next (&gsi); >> + while (!gsi_end_p (gsi) >> + && is_gimple_debug (gsi_stmt (gsi))) >> + gsi_next (&gsi); >> + >> + /* Only a singleton if there's no next nondebug gimple. */ >> + return gsi_end_p (gsi); >> +} >> + >> #endif /* GCC_GIMPLE_ITERATOR_H */ >> diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c >> index 02f44eb..c4b83fe 100644 >> --- a/gcc/tree-parloops.c >> +++ b/gcc/tree-parloops.c >> @@ -1769,8 +1769,8 @@ try_transform_to_exit_first_loop_alt (struct loop *loop, >> tree nit) >> { >> /* Check whether the latch contains a single statement. */ >> - if (!gimple_seq_singleton_p (bb_seq (loop->latch))) >> - return true; >> + if (!gimple_seq_nondebug_singleton_p (bb_seq (loop->latch))) >> + return false; >> >> /* Check whether the latch contains the loop iv increment. */ >> edge back = single_succ_edge (loop->latch); >> -- >> 1.9.1 >>
Fix try_transform_to_exit_first_loop_alt 2015-06-06 Tom de Vries <tom@codesourcery.com> PR tree-optimization/66442 * gimple-iterator.h (gimple_seq_nondebug_singleton_p): Add function. * tree-parloops.c (try_transform_to_exit_first_loop_alt): Return false if the loop latch is not a singleton. Use gimple_seq_nondebug_singleton_p instead of gimple_seq_singleton_p. --- gcc/gimple-iterator.h | 29 +++++++++++++++++++++++++++++ gcc/tree-parloops.c | 4 ++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h index 87e943a..76fa456 100644 --- a/gcc/gimple-iterator.h +++ b/gcc/gimple-iterator.h @@ -345,4 +345,33 @@ gsi_seq (gimple_stmt_iterator i) return *i.seq; } +/* Determine whether SEQ is a nondebug singleton. */ + +static inline bool +gimple_seq_nondebug_singleton_p (gimple_seq seq) +{ + gimple_stmt_iterator gsi; + + /* Find a nondebug gimple. */ + gsi.ptr = gimple_seq_first (seq); + gsi.seq = &seq; + gsi.bb = NULL; + while (!gsi_end_p (gsi) + && is_gimple_debug (gsi_stmt (gsi))) + gsi_next (&gsi); + + /* No nondebug gimple found, not a singleton. */ + if (gsi_end_p (gsi)) + return false; + + /* Find a next nondebug gimple. */ + gsi_next (&gsi); + while (!gsi_end_p (gsi) + && is_gimple_debug (gsi_stmt (gsi))) + gsi_next (&gsi); + + /* Only a singleton if there's no next nondebug gimple. */ + return gsi_end_p (gsi); +} + #endif /* GCC_GIMPLE_ITERATOR_H */ diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c index 02f44eb..c4b83fe 100644 --- a/gcc/tree-parloops.c +++ b/gcc/tree-parloops.c @@ -1769,8 +1769,8 @@ try_transform_to_exit_first_loop_alt (struct loop *loop, tree nit) { /* Check whether the latch contains a single statement. */ - if (!gimple_seq_singleton_p (bb_seq (loop->latch))) - return true; + if (!gimple_seq_nondebug_singleton_p (bb_seq (loop->latch))) + return false; /* Check whether the latch contains the loop iv increment. */ edge back = single_succ_edge (loop->latch); -- 1.9.1