Message ID | 1344978713.31850.6.camel@gnopaine |
---|---|
State | New |
Headers | show |
On Tue, Aug 14, 2012 at 2:11 PM, William J. Schmidt <wschmidt@linux.vnet.ibm.com> wrote: > Replace the once vacuously true, and now vacuously false, test for > existence of a conditional move instruction for a given mode, with one > that actually checks what it's supposed to. Add a test case so we don't > miss such things in future. > > The test is powerpc-specific. It would be good to have an i386 version > of the test as well, if someone can help with that. > > Bootstrapped and tested on powerpc64-unknown-linux-gnu with no new > regressions. Ok for trunk? Here is one which can go into gcc.target/mips : /* { dg-do compile } */ /* { dg-options "-O2 -fdump-tree-phiopt-details" } */ typedef struct s { int v; int b; struct s *l; struct s *r; } S; int foo(S *s) { S *this; S *next; this = s; if (this->b) next = this->l; else next = this->r; return next->v; } /* { dg-final { scan-tree-dump "Hoisting adjacent loads" "phiopt1" } } */ /* { dg-final { cleanup-tree-dump "phiopt1" } } */
On Tue, Aug 14, 2012 at 2:15 PM, Andrew Pinski <pinskia@gmail.com> wrote: > On Tue, Aug 14, 2012 at 2:11 PM, William J. Schmidt > <wschmidt@linux.vnet.ibm.com> wrote: >> Replace the once vacuously true, and now vacuously false, test for >> existence of a conditional move instruction for a given mode, with one >> that actually checks what it's supposed to. Add a test case so we don't >> miss such things in future. >> >> The test is powerpc-specific. It would be good to have an i386 version >> of the test as well, if someone can help with that. >> >> Bootstrapped and tested on powerpc64-unknown-linux-gnu with no new >> regressions. Ok for trunk? > > Here is one which can go into gcc.target/mips : > /* { dg-do compile } */ > /* { dg-options "-O2 -fdump-tree-phiopt-details" } */ Sorry the dg-options should be: /* { dg-options "-O2 -fdump-tree-phiopt-details isa>=4" } */ Thanks, Andrew > > typedef struct s { > int v; > int b; > struct s *l; > struct s *r; > } S; > > > int foo(S *s) > { > S *this; > S *next; > > this = s; > if (this->b) > next = this->l; > else > next = this->r; > > return next->v; > } > > /* { dg-final { scan-tree-dump "Hoisting adjacent loads" "phiopt1" } } */ > /* { dg-final { cleanup-tree-dump "phiopt1" } } */
Thanks, Andrew! Bill On Tue, 2012-08-14 at 14:17 -0700, Andrew Pinski wrote: > On Tue, Aug 14, 2012 at 2:15 PM, Andrew Pinski <pinskia@gmail.com> wrote: > > On Tue, Aug 14, 2012 at 2:11 PM, William J. Schmidt > > <wschmidt@linux.vnet.ibm.com> wrote: > >> Replace the once vacuously true, and now vacuously false, test for > >> existence of a conditional move instruction for a given mode, with one > >> that actually checks what it's supposed to. Add a test case so we don't > >> miss such things in future. > >> > >> The test is powerpc-specific. It would be good to have an i386 version > >> of the test as well, if someone can help with that. > >> > >> Bootstrapped and tested on powerpc64-unknown-linux-gnu with no new > >> regressions. Ok for trunk? > > > > Here is one which can go into gcc.target/mips : > > /* { dg-do compile } */ > > /* { dg-options "-O2 -fdump-tree-phiopt-details" } */ > > Sorry the dg-options should be: > /* { dg-options "-O2 -fdump-tree-phiopt-details isa>=4" } */ > > Thanks, > Andrew > > > > > typedef struct s { > > int v; > > int b; > > struct s *l; > > struct s *r; > > } S; > > > > > > int foo(S *s) > > { > > S *this; > > S *next; > > > > this = s; > > if (this->b) > > next = this->l; > > else > > next = this->r; > > > > return next->v; > > } > > > > /* { dg-final { scan-tree-dump "Hoisting adjacent loads" "phiopt1" } } */ > > /* { dg-final { cleanup-tree-dump "phiopt1" } } */ >
On Tue, 14 Aug 2012, William J. Schmidt wrote: > Replace the once vacuously true, and now vacuously false, test for > existence of a conditional move instruction for a given mode, with one > that actually checks what it's supposed to. Add a test case so we don't > miss such things in future. > > The test is powerpc-specific. It would be good to have an i386 version > of the test as well, if someone can help with that. > > Bootstrapped and tested on powerpc64-unknown-linux-gnu with no new > regressions. Ok for trunk? Ok if you also drop in the testcase from Andrew. Thanks, Richard. > Thanks, > Bill > > > gcc: > > 2012-08-13 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > PR tree-optimization/54240 > * tree-ssa-phiopt.c (hoist_adjacent_loads): Correct test for > existence of conditional move with given mode. > > > gcc/testsuite: > > 2012-08-13 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > PR tree-optimization/54240 > * gcc.target/powerpc/pr54240.c: New test. > > > Index: gcc/testsuite/gcc.target/powerpc/pr54240.c > =================================================================== > --- gcc/testsuite/gcc.target/powerpc/pr54240.c (revision 0) > +++ gcc/testsuite/gcc.target/powerpc/pr54240.c (revision 0) > @@ -0,0 +1,27 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -misel -fdump-tree-phiopt-details" } */ > + > +typedef struct s { > + int v; > + int b; > + struct s *l; > + struct s *r; > +} S; > + > + > +int foo(S *s) > +{ > + S *this; > + S *next; > + > + this = s; > + if (this->b) > + next = this->l; > + else > + next = this->r; > + > + return next->v; > +} > + > +/* { dg-final { scan-tree-dump "Hoisting adjacent loads" "phiopt1" } } */ > +/* { dg-final { cleanup-tree-dump "phiopt1" } } */ > Index: gcc/tree-ssa-phiopt.c > =================================================================== > --- gcc/tree-ssa-phiopt.c (revision 190305) > +++ gcc/tree-ssa-phiopt.c (working copy) > @@ -1843,7 +1843,8 @@ hoist_adjacent_loads (basic_block bb0, basic_block > > /* Check the mode of the arguments to be sure a conditional move > can be generated for it. */ > - if (!optab_handler (cmov_optab, TYPE_MODE (TREE_TYPE (arg1)))) > + if (optab_handler (movcc_optab, TYPE_MODE (TREE_TYPE (arg1))) > + == CODE_FOR_nothing) > continue; > > /* Both statements must be assignments whose RHS is a COMPONENT_REF. */ > > >
Index: gcc/testsuite/gcc.target/powerpc/pr54240.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/pr54240.c (revision 0) +++ gcc/testsuite/gcc.target/powerpc/pr54240.c (revision 0) @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -misel -fdump-tree-phiopt-details" } */ + +typedef struct s { + int v; + int b; + struct s *l; + struct s *r; +} S; + + +int foo(S *s) +{ + S *this; + S *next; + + this = s; + if (this->b) + next = this->l; + else + next = this->r; + + return next->v; +} + +/* { dg-final { scan-tree-dump "Hoisting adjacent loads" "phiopt1" } } */ +/* { dg-final { cleanup-tree-dump "phiopt1" } } */ Index: gcc/tree-ssa-phiopt.c =================================================================== --- gcc/tree-ssa-phiopt.c (revision 190305) +++ gcc/tree-ssa-phiopt.c (working copy) @@ -1843,7 +1843,8 @@ hoist_adjacent_loads (basic_block bb0, basic_block /* Check the mode of the arguments to be sure a conditional move can be generated for it. */ - if (!optab_handler (cmov_optab, TYPE_MODE (TREE_TYPE (arg1)))) + if (optab_handler (movcc_optab, TYPE_MODE (TREE_TYPE (arg1))) + == CODE_FOR_nothing) continue; /* Both statements must be assignments whose RHS is a COMPONENT_REF. */