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