| Message ID | 87911474-3353-c0dc-4d8f-b2ac15e8db12@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 2016.11.18 at 10:27 -0600, Bill Schmidt wrote: > =================================================================== > --- gcc/testsuite/gcc.dg/tree-ssa/pr78413.c (revision 0) > +++ gcc/testsuite/gcc.dg/tree-ssa/pr78413.c (working copy) > @@ -0,0 +1,35 @@ > +/* PR78413. These previously failed in tree if-conversion due to a loop > + latch with multiple predecessors that the code did not anticipate. */ > +/* { dg-do compile } */ > +/* { dg-options "-O3 -ffast-math" } */ Please add -fno-strict-aliasing, otherwise fn1() wouldn't ICE. > +extern long long int llrint(double x); > +int a; > +double b; > +__attribute__((cold)) void decode_init() { > + int c, d = 0; > + for (; d < 12; d++) { > + if (d) > + b = 0; > + c = 0; > + for (; c < 6; c++) > + a = b ? llrint(b) : 0; > + } > +} > + > +struct S { > + _Bool bo; > +}; > +int a, bb, c, d; > +void fn1() { > + do > + do > + do { > + struct S *e = (struct S *)1; > + do > + bb = a / (e->bo ? 2 : 1); > + while (bb); > + } while (0); > + while (d); > + while (c); > +}
> On Nov 18, 2016, at 10:33 AM, Markus Trippelsdorf <markus@trippelsdorf.de> wrote: > > On 2016.11.18 at 10:27 -0600, Bill Schmidt wrote: >> =================================================================== >> --- gcc/testsuite/gcc.dg/tree-ssa/pr78413.c (revision 0) >> +++ gcc/testsuite/gcc.dg/tree-ssa/pr78413.c (working copy) >> @@ -0,0 +1,35 @@ >> +/* PR78413. These previously failed in tree if-conversion due to a loop >> + latch with multiple predecessors that the code did not anticipate. */ >> +/* { dg-do compile } */ >> +/* { dg-options "-O3 -ffast-math" } */ > > Please add -fno-strict-aliasing, otherwise fn1() wouldn't ICE. Whoops. Yes indeed, sorry I missed the flag difference for the second failure. Bill
On Fri, Nov 18, 2016 at 5:27 PM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote: > Hi, > > The if-conversion patch for PR77848 missed a case where an outer loop > should not be versioned for vectorization; this was caught by an assert > in tests recorded in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78413. > This patch fixes the problem by ensuring that both inner and outer loop > latches have a single predecessor before versioning an outer loop. > > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no > regressions. Is this ok for trunk? Ok (with the suggested testcase adjustment). Maybe as a followup we can factor out a common predicate usable by the vectorizer and if-conversion. I'll see if I can do that. Thanks, Richard. > Thanks, > Bill > > > [gcc] > > 2016-11-18 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > PR tree-optimization/78413 > * tree-if-conv.c (versionable_outer_loop_p): Require that both > inner and outer loop latches have single predecessors. > > [gcc/testsuite] > > 2016-11-18 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > PR tree-optimization/78413 > * gcc.dg/tree-ssa/pr78413.c: New test. > > > Index: gcc/testsuite/gcc.dg/tree-ssa/pr78413.c > =================================================================== > --- gcc/testsuite/gcc.dg/tree-ssa/pr78413.c (revision 0) > +++ gcc/testsuite/gcc.dg/tree-ssa/pr78413.c (working copy) > @@ -0,0 +1,35 @@ > +/* PR78413. These previously failed in tree if-conversion due to a loop > + latch with multiple predecessors that the code did not anticipate. */ > +/* { dg-do compile } */ > +/* { dg-options "-O3 -ffast-math" } */ > + > +extern long long int llrint(double x); > +int a; > +double b; > +__attribute__((cold)) void decode_init() { > + int c, d = 0; > + for (; d < 12; d++) { > + if (d) > + b = 0; > + c = 0; > + for (; c < 6; c++) > + a = b ? llrint(b) : 0; > + } > +} > + > +struct S { > + _Bool bo; > +}; > +int a, bb, c, d; > +void fn1() { > + do > + do > + do { > + struct S *e = (struct S *)1; > + do > + bb = a / (e->bo ? 2 : 1); > + while (bb); > + } while (0); > + while (d); > + while (c); > +} > Index: gcc/tree-if-conv.c > =================================================================== > --- gcc/tree-if-conv.c (revision 242551) > +++ gcc/tree-if-conv.c (working copy) > @@ -2575,6 +2575,8 @@ version_loop_for_if_conversion (struct loop *loop) > - The loop has a single exit. > - The loop header has a single successor, which is the inner > loop header. > + - Each of the inner and outer loop latches have a single > + predecessor. > - The loop exit block has a single predecessor, which is the > inner loop's exit block. */ > > @@ -2586,7 +2588,9 @@ versionable_outer_loop_p (struct loop *loop) > || loop->inner->next > || !single_exit (loop) > || !single_succ_p (loop->header) > - || single_succ (loop->header) != loop->inner->header) > + || single_succ (loop->header) != loop->inner->header > + || !single_pred_p (loop->latch) > + || !single_pred_p (loop->inner->latch)) > return false; > > basic_block outer_exit = single_pred (loop->latch); >
Index: gcc/testsuite/gcc.dg/tree-ssa/pr78413.c =================================================================== --- gcc/testsuite/gcc.dg/tree-ssa/pr78413.c (revision 0) +++ gcc/testsuite/gcc.dg/tree-ssa/pr78413.c (working copy) @@ -0,0 +1,35 @@ +/* PR78413. These previously failed in tree if-conversion due to a loop + latch with multiple predecessors that the code did not anticipate. */ +/* { dg-do compile } */ +/* { dg-options "-O3 -ffast-math" } */ + +extern long long int llrint(double x); +int a; +double b; +__attribute__((cold)) void decode_init() { + int c, d = 0; + for (; d < 12; d++) { + if (d) + b = 0; + c = 0; + for (; c < 6; c++) + a = b ? llrint(b) : 0; + } +} + +struct S { + _Bool bo; +}; +int a, bb, c, d; +void fn1() { + do + do + do { + struct S *e = (struct S *)1; + do + bb = a / (e->bo ? 2 : 1); + while (bb); + } while (0); + while (d); + while (c); +} Index: gcc/tree-if-conv.c =================================================================== --- gcc/tree-if-conv.c (revision 242551) +++ gcc/tree-if-conv.c (working copy) @@ -2575,6 +2575,8 @@ version_loop_for_if_conversion (struct loop *loop) - The loop has a single exit. - The loop header has a single successor, which is the inner loop header. + - Each of the inner and outer loop latches have a single + predecessor. - The loop exit block has a single predecessor, which is the inner loop's exit block. */ @@ -2586,7 +2588,9 @@ versionable_outer_loop_p (struct loop *loop) || loop->inner->next || !single_exit (loop) || !single_succ_p (loop->header) - || single_succ (loop->header) != loop->inner->header) + || single_succ (loop->header) != loop->inner->header + || !single_pred_p (loop->latch) + || !single_pred_p (loop->inner->latch)) return false; basic_block outer_exit = single_pred (loop->latch);