Submitted by Andrew Pinski on July 24, 2012, 3:50 p.m.

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" } } */