On Sat, 9 Nov 2013, Tom de Vries wrote:
> Richard,
>
> Consider the test-case test.c:
> ...
> int z;
> int x;
>
> void
> f (int c, int d)
> {
> if (c)
> z = 5;
> else
> {
> if (d)
> x = 4;
> z = 5;
> }
> }
> ...
>
> Atm, we don't tail-merge the 'z = 5' blocks, because gimple_equal_p returns
> false for the 'z = 5' statements. The relevant code is this:
> ...
> if (TREE_CODE (lhs1) != SSA_NAME
> && TREE_CODE (lhs2) != SSA_NAME)
> return (vn_valueize (gimple_vdef (s1))
> == vn_valueize (gimple_vdef (s2)));
> ...
> The vdefs of the 'z = 5' statements are different, because the incoming vuses
> are different.
>
> This patch handles GIMPLE_ASSIGNs with different vuse in gimple_equal_p, by
> doing a structural comparison.
>
> Bootstrapped and regtested on x86_64.
>
> OK for trunk?
Comments inline
diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c
index 98b5882..43516a7 100644
--- a/gcc/tree-ssa-tail-merge.c
+++ b/gcc/tree-ssa-tail-merge.c
@@ -1173,8 +1173,47 @@ gimple_equal_p (same_succ same_succ, gimple s1, gimple
s2)
lhs2 = gimple_get_lhs (s2);
if (TREE_CODE (lhs1) != SSA_NAME
&& TREE_CODE (lhs2) != SSA_NAME)
- return (vn_valueize (gimple_vdef (s1))
- == vn_valueize (gimple_vdef (s2)));
+ {
+ /* If the vdef is the same, it's the same statement. */
+ if (vn_valueize (gimple_vdef (s1))
+ == vn_valueize (gimple_vdef (s2)))
+ return true;
+
+ /* If the vdef is not the same but the vuse is the same, it's not the
+ same stmt. */
+ if (vn_valueize (gimple_vuse (s1))
+ == vn_valueize (gimple_vuse (s2)))
+ return false;
What's the logic behind this? We want to use VN to get more "positive"
results - doing a negative early out looks suspicious to me ...
+ /* If the vdef is not the same and the vuse is not the same, it might
be
+ same stmt. */
+
+ /* Test for structural equality. */
+ if (gimple_assign_rhs_code (s1) != gimple_assign_rhs_code (s1)
s2
+ || (gimple_assign_nontemporal_move_p (s1)
+ != gimple_assign_nontemporal_move_p (s2)))
I don't think we should care (it'll be false - a later pass sets it,
it's an optimization hint, not a correctness issue). More interesting
would be to assert that has_volatile_ops is the same if the operands
turned out to be the same.
+ return false;
+
+ if (!operand_equal_p (lhs1, lhs2, 0))
+ return false;
+
+ t1 = gimple_assign_rhs1 (s1);
+ t2 = gimple_assign_rhs1 (s2);
+ if (!gimple_operand_equal_value_p (t1, t2))
+ return false;
+
+ t1 = gimple_assign_rhs2 (s1);
+ t2 = gimple_assign_rhs2 (s2);
+ if (!gimple_operand_equal_value_p (t1, t2))
+ return false;
+
+ t1 = gimple_assign_rhs3 (s1);
+ t2 = gimple_assign_rhs3 (s2);
+ if (!gimple_operand_equal_value_p (t1, t2))
+ return false;
for (i = 1; i < gimple_num_ops (s1); ++i)
t1 = gimple_op (s1, i);
...
but I think you should only compare rhs1 and thus only handle
GIMPLE_ASSIGN_SINGLEs this way - the others have a SSA name
lhs.
That makes the whole thing just
if (TREE_CODE (lhs1) != SSA_NAME
&& TREE_CODE (lhs2) != SSA_NAME)
{
if (vn_valueize (gimple_vdef (s1))
== vn_valueize (gimple_vdef (s2)))
return true;
return operand_equal_p (lhs1, lhs2, 0)
&& gimple_operand_equal_value_p (gimple_assign_rhs1 (s1),
gimple_assign_rhs2 (s2));
}
Ok with doing it this way.
Thanks,
Richard.
+ /* Same structure. */
+ return true;
+ }
else if (TREE_CODE (lhs1) == SSA_NAME
&& TREE_CODE (lhs2) == SSA_NAME)
return vn_valueize (lhs1) == vn_valueize (lhs2);
