On Thu, 5 Jul 2007, Roman Zippel wrote:

> Hi,
> 
> On Thu, 5 Jul 2007, Richard Guenther wrote:
> 
> > If there is anything to fix, then all those variants should produce
> > the same code, not just foo3 and foo4.  So for these cases we should
> > make sure that value-numbering sees them as computing the same value
> > and extend combine to choose the instructions with the least cost.
> > 
> > Changing fold isn't a real fix.  It's a workaround for a specific
> > testcase.
> 
> What do you suggest now specifically?
> combine isn't the problem here, at the time we reach RTL this should 
> already be done. Your patch only catches specific cases and pessimises 
> others.

Well, that's always the nature of any canonicalization.

The following actually makes SCCVN notice that tmp1 == tmp2 in

int foo3(int x, int y)
{
        int tmp1 = (x * y) + (y * 5);
        int tmp2;
        x *= y;
        tmp2 = x + (y * 5);
        return tmp1 == tmp2;
}

of course only tested on this particular testcase.  It just shows
that it is possible to fix this in a generic way.

Richard.

Index: tree-ssa-sccvn.c
===================================================================
--- tree-ssa-sccvn.c    (revision 126369)
+++ tree-ssa-sccvn.c    (working copy)
@@ -1387,6 +1389,8 @@ simplify_binary_expression (tree rhs)
        op0 = valueize_expr (VN_INFO (op0)->expr);
       else if (SSA_VAL (op0) != VN_TOP && SSA_VAL (op0) != op0)
        op0 = VN_INFO (op0)->valnum;      
+      else
+       op0 = valueize_expr (VN_INFO (op0)->expr);
     }
 
   if (TREE_CODE (op1) == SSA_NAME)
@@ -1395,7 +1399,14 @@ simplify_binary_expression (tree rhs)
        op1 = valueize_expr (VN_INFO (op1)->expr);
       else if (SSA_VAL (op1) != VN_TOP && SSA_VAL (op1) != op1)
        op1 = VN_INFO (op1)->valnum;
+      else
+       op1 = valueize_expr (VN_INFO (op1)->expr);
     }
+
+  if (op0 == TREE_OPERAND (rhs, 0)
+      && op1 == TREE_OPERAND (rhs, 1))
+    return rhs;
+
   result = fold_binary (TREE_CODE (rhs), TREE_TYPE (rhs), op0, op1);
 
   /* Make sure result is not a complex expression consiting
@@ -1423,6 +1434,16 @@ simplify_binary_expression (tree rhs)
              {
                tree op0 = TREE_OPERAND (result, 0);
                tree op1 = TREE_OPERAND (result, 1);
+               tree tmp0, tmp1;
+               if (EXPR_P (op0)
+                   && (tmp0 = vn_binary_op_lookup (op0)))
+                 {
+                   tmp1 = vn_binary_op_lookup (build2 (TREE_CODE (result), 
TREE_TYPE (result), tmp0, op1));
+                   if (tmp1)
+                     return tmp1;
+                 }
+               if ((tmp0 = vn_binary_op_lookup (result)))
+                 return tmp0;
                if (!EXPR_P (op0) && !EXPR_P (op1))
                  return result;
              }
@@ -1607,7 +1637,7 @@ visit_use (tree use)
                 even if they were optimistically constant. */
                 
              VN_INFO (lhs)->has_constants = false;
-             VN_INFO (lhs)->expr = lhs;
+             VN_INFO (lhs)->expr = rhs;
            }
 
          if (TREE_CODE (lhs) == SSA_NAME

Reply via email to