Re: [PATCH] Fix PR 52631 (VN does not use simplified expression for lookup)

2012-08-20 Thread Richard Guenther 
On Mon, Aug 20, 2012 at 6:49 AM, Andrew Pinski
andrew.pin...@caviumnetworks.com wrote:
 On Wed, Jul 25, 2012 at 4:39 AM, Richard Guenther
 richard.guent...@gmail.com wrote:
 On Tue, Jul 24, 2012 at 5:50 PM, Andrew Pinski
 andrew.pin...@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.

Re: [PATCH] Fix PR 52631 (VN does not use simplified expression for lookup)

2012-08-19 Thread Andrew Pinski 
On Wed, Jul 25, 2012 at 4:39 AM, Richard Guenther
richard.guent...@gmail.com wrote:
 On Tue, Jul 24, 2012 at 5:50 PM, Andrew Pinski
 andrew.pin...@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.

Re: [PATCH] Fix PR 52631 (VN does not use simplified expression for lookup)

2012-07-25 Thread Richard Guenther 
On Tue, Jul 24, 2012 at 5:50 PM, Andrew Pinski
andrew.pin...@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.

[PATCH] Fix PR 52631 (VN does not use simplified expression for lookup)

2012-07-24 Thread Andrew Pinski 
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.
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 } } */