| Message ID | CA+=Sn1nE=aAEWmLCu_-QLqevzXHbBZLzxPp0SKhNsRo1ajM9KA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 24, 2012 at 5:50 PM, Andrew Pinski <andrew.pinski@caviumnetworks.com> wrote: > Hi, > Before tuples was introduced, VN used to lookup the simplified > expression to see if it was available already and use that instead of > the non simplified one. This patch adds the support back to VN to do > exactly that. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. I think this should be done for all RHS and SSA name LHS, not only for UNARY/BINARY/TERNARY - because even for SINGLE rhs we can end up simplifying (for REALPART_EXPR for example which we handle as nary, too). I think we constrain try_to_simplify enough so that + /* First try to lookup the simplified expression. */ + if (simplified && valid_gimple_rhs_p (simplified)) + { + tree result = vn_nary_op_lookup (simplified, NULL); + if (result) + { + changed = set_ssa_val_to (lhs, result); + goto done; + } + changed = set_ssa_val_to (lhs, lhs); + vn_nary_op_insert (simplified, lhs); + } switch (get_gimple_rhs_class (code)) { case GIMPLE_UNARY_RHS: case GIMPLE_BINARY_RHS: ... should work. As you also insert the simplified variant I think we really (finally) want to have a valid_nary_op routine rather than relying on valid_gimple_rhs_p which is way too generic. Thanks, Richard. > Thanks, > Andrew Pinski > > ChangeLog: > > * tree-ssa-sccvn.c (visit_use): Look up the simplified > expression before visting the original one. > > * gcc.dg/tree-ssa/ssa-fre-9.c: Update the number of > eliminatations that happen.
On Wed, Jul 25, 2012 at 4:39 AM, Richard Guenther <richard.guenther@gmail.com> wrote: > On Tue, Jul 24, 2012 at 5:50 PM, Andrew Pinski > <andrew.pinski@caviumnetworks.com> wrote: >> Hi, >> Before tuples was introduced, VN used to lookup the simplified >> expression to see if it was available already and use that instead of >> the non simplified one. This patch adds the support back to VN to do >> exactly that. >> >> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > I think this should be done for all RHS and SSA name LHS, not only > for UNARY/BINARY/TERNARY - because even for SINGLE rhs we > can end up simplifying (for REALPART_EXPR for example which we > handle as nary, too). I think we constrain try_to_simplify enough > so that > > + /* First try to lookup the simplified expression. */ > + if (simplified && valid_gimple_rhs_p (simplified)) > + { > + tree result = vn_nary_op_lookup (simplified, NULL); > + if (result) > + { > + changed = set_ssa_val_to (lhs, result); > + goto done; > + } > + changed = set_ssa_val_to (lhs, lhs); > + vn_nary_op_insert (simplified, lhs); > + } > switch (get_gimple_rhs_class (code)) > { > case GIMPLE_UNARY_RHS: > case GIMPLE_BINARY_RHS: > ... > > should work. As you also insert the simplified variant I think we really > (finally) want to have a valid_nary_op routine rather than relying on > valid_gimple_rhs_p which is way too generic. I don't see valid_gimple_rhs_p being that generic as it checks to make sure the operands of the gimple are valid. Maybe I am missing something here though. Thanks, Andrew Pinski > > Thanks, > Richard. > >> Thanks, >> Andrew Pinski >> >> ChangeLog: >> >> * tree-ssa-sccvn.c (visit_use): Look up the simplified >> expression before visting the original one. >> >> * gcc.dg/tree-ssa/ssa-fre-9.c: Update the number of >> eliminatations that happen.
On Mon, Aug 20, 2012 at 6:49 AM, Andrew Pinski <andrew.pinski@caviumnetworks.com> wrote: > On Wed, Jul 25, 2012 at 4:39 AM, Richard Guenther > <richard.guenther@gmail.com> wrote: >> On Tue, Jul 24, 2012 at 5:50 PM, Andrew Pinski >> <andrew.pinski@caviumnetworks.com> wrote: >>> Hi, >>> Before tuples was introduced, VN used to lookup the simplified >>> expression to see if it was available already and use that instead of >>> the non simplified one. This patch adds the support back to VN to do >>> exactly that. >>> >>> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. >> >> I think this should be done for all RHS and SSA name LHS, not only >> for UNARY/BINARY/TERNARY - because even for SINGLE rhs we >> can end up simplifying (for REALPART_EXPR for example which we >> handle as nary, too). I think we constrain try_to_simplify enough >> so that >> >> + /* First try to lookup the simplified expression. */ >> + if (simplified && valid_gimple_rhs_p (simplified)) >> + { >> + tree result = vn_nary_op_lookup (simplified, NULL); >> + if (result) >> + { >> + changed = set_ssa_val_to (lhs, result); >> + goto done; >> + } >> + changed = set_ssa_val_to (lhs, lhs); >> + vn_nary_op_insert (simplified, lhs); >> + } >> switch (get_gimple_rhs_class (code)) >> { >> case GIMPLE_UNARY_RHS: >> case GIMPLE_BINARY_RHS: >> ... >> >> should work. As you also insert the simplified variant I think we really >> (finally) want to have a valid_nary_op routine rather than relying on >> valid_gimple_rhs_p which is way too generic. > > I don't see valid_gimple_rhs_p being that generic as it checks to make > sure the operands of the gimple are valid. > Maybe I am missing something here though. valid_gimple_rhs_p checks what it says. But what we want to know is whether the rhs is valid for a SCCVN NARY. Those are not the same. Richard. > Thanks, > Andrew Pinski > > >> >> Thanks, >> Richard. >> >>> Thanks, >>> Andrew Pinski >>> >>> ChangeLog: >>> >>> * tree-ssa-sccvn.c (visit_use): Look up the simplified >>> expression before visting the original one. >>> >>> * gcc.dg/tree-ssa/ssa-fre-9.c: Update the number of >>> eliminatations that happen.
Index: gcc/tree-ssa-sccvn.c =================================================================== --- gcc/tree-ssa-sccvn.c (revision 189800) +++ gcc/tree-ssa-sccvn.c (working copy) @@ -3366,6 +3366,19 @@ visit_use (tree use) case GIMPLE_UNARY_RHS: case GIMPLE_BINARY_RHS: case GIMPLE_TERNARY_RHS: + /* First try to lookup the simplified expression. */ + if (simplified && valid_gimple_rhs_p (simplified)) + { + tree result = vn_nary_op_lookup (simplified, NULL); + if (result) + { + changed = set_ssa_val_to (lhs, result); + goto done; + } + changed = set_ssa_val_to (lhs, lhs); + vn_nary_op_insert (simplified, lhs); + } + /* Otherwise visit the original statement. */ changed = visit_nary_op (lhs, stmt); break; case GIMPLE_SINGLE_RHS: Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-9.c =================================================================== --- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-9.c (revision 189800) +++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-9.c (working copy) @@ -23,6 +23,6 @@ void __frame_state_for1 (volatile char * } } -/* { dg-final { scan-tree-dump-times "Eliminated: 1" 2 "fre1" } } */ +/* { dg-final { scan-tree-dump-times "Eliminated: 2" 2 "fre1" } } */ /* { dg-final { scan-tree-dump-times "Insertions: 1" 2 "fre1" } } */ /* { dg-final { cleanup-tree-dump "fre1" } } */
Hi, Before tuples was introduced, VN used to lookup the simplified expression to see if it was available already and use that instead of the non simplified one. This patch adds the support back to VN to do exactly that. OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. Thanks, Andrew Pinski ChangeLog: * tree-ssa-sccvn.c (visit_use): Look up the simplified expression before visting the original one. * gcc.dg/tree-ssa/ssa-fre-9.c: Update the number of eliminatations that happen.