unsigned f(unsigned a)
unsigned b = a >> 31;
--- CUT ---
Even though we have a simplified expression of a>>31 for b, VN does not use it.
SCC consists of: a_1(D)
Setting value number of a_1(D) to a_1(D)
SCC consists of: b_2
Value numbering b_2 stmt = b_2 = a_1(D) >> 31;
Setting value number of b_2 to b_2 (changed)
SCC consists of: D.1708_3
Value numbering D.1708_3 stmt = D.1708_3 = b_2 & 1;
RHS b_2 & 1 simplified to a_1(D) >> 31 has constants 1
Setting value number of D.1708_3 to D.1708_3 (changed)
This is a regression from 4.3.
I am working on a fix. Note I did not find this because a performance issue but rather I found this while working on my gimple SSA combiner/simplifer patches.
Created attachment 26927 [details]
Patch which fixes the problem
Hmm, but then you'd pessimize the case where b_2 & 1 were available? Thus,
don't you need to do the lookup with the original expression anyway if the
lookup for the simplified expression fails? Oh, and doesn't the testcase
show a missed canonicalization?
(In reply to comment #3)
> Hmm, but then you'd pessimize the case where b_2 & 1 were available? Thus,
> don't you need to do the lookup with the original expression anyway if the
> lookup for the simplified expression fails? Oh, and doesn't the testcase
> show a missed canonicalization?
My patch just gets us back to where we were before tuples.
Before tuples we would do:
else if (simplified)
if (TREE_CODE (lhs) == SSA_NAME)
VN_INFO (lhs)->has_constants = expr_has_constants (simplified);
/* We have to unshare the expression or else
valuizing may change the IL stream. */
VN_INFO (lhs)->expr = unshare_expr (simplified);
rhs = simplified;
And then use rhs in the switch statement for visit_unary_op/visit_binary_op .
I don't see a missed canonicalization anywhere as we are combing (a>>31)&1 and then calling fold on it and it returns a>>31 which is the correct thing to do. The main issue is we never do a lookup on a>>31 after we do the simplification, we only do it on the b&1.
Let me see if I can understand the question about b&1 being available, a testcase would be something like:
unsigned f(unsigned a, unsigned b, unsigned c)
unsigned d = c&1;
c = a>>31;
return (c & 1) + d;
--- CUT ---
PRE produces (with both the patch and before):
d_3 = c_2(D) & 1;
if (b_4(D) != 0)
goto <bb 3>;
goto <bb 5>;
goto <bb 4>;
c_6 = a_5(D) >> 31;
pretmp.3_11 = c_6 & 1;
# c_1 = PHI <c_2(D)(5), c_6(3)>
# prephitmp.4_12 = PHI <d_3(5), pretmp.3_11(3)>
D.1713_7 = prephitmp.4_12;
D.1712_8 = D.1713_7 + d_3;
Which means we don't even do the simplifications while doing a PRE anyways.
Ok, I have a patch which only tries to looking up the simplified expression and then if we find it, use it, otherwise use the original expression. I now understand why this makes a difference (for FRE it does not though).
Here is the simpler patch:
--- tree-ssa-sccvn.c (revision 185559)
+++ tree-ssa-sccvn.c (working copy)
@@ -3295,6 +3295,17 @@ visit_use (tree use)
+ /* 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;
+ /* Otherwise visit the original statement. */
switch (get_gimple_rhs_class (code))
---- CUT -----
* tree-ssa-sccvn.c (visit_use): Before looking up the original statement, try looking up the simplified expression.
That looks good apart from the use of valid_gimple_rhs_p which allows too much.
I'd rather use get_gimple_rhs_class (TREE_CODE (simplified)) and allow
GIMPLE_UNARY_RHS, GIMPLE_BINARY_RHS and GIMPLE_TERNARY_RHS here.
GCC 4.7.1 is being released, adjusting target milestone.
GCC 4.7.2 has been released.
Date: Sun Jan 20 05:00:56 2013
New Revision: 195318
* tree-ssa-sccvn (visit_use): Before looking up the original
statement, try looking up the simplified expression.
* tree-ssa/pr52631.c: New test.
* tree-ssa/ssa-fre-9: Update expected output.
Fixed on trunk.
Date: Mon Mar 4 11:12:30 2013
New Revision: 196428
* gcc.dg/tree-ssa/pr52631.c: Fix 16-bit int.