On 04/19/2016 11:50 AM, Patrick Palka wrote:
1. This patch introduces a "regression" in gcc.dg/tree-ssa/ssa-thread-11.c
in that we no longer perform FSM threading during vrp2 but instead we
detect two new jump threading opportunities during vrp1. Not sure if
the new code is better but it is shorter. I wonder how this should be
resolved...
Definitely not a regression. As you note we thread two jumps earlier
utilizing your new code. With the old code we're dependent upon other
simplifications occurring which eventually exposes the FSM threads that
the test is checking for in vrp2.
I think we just want to remove the test for FSM jump threads in VRP2.
We get coverage for your test via ssa-thread-14. That should just leave
the verification that we do not have irreducible loops at the end of
VRP2 in ssa-thread-11. I'll make that change.
I do see what appears to be a missed jump thread, but that's not
affected positively or negatively by your change. There's a reasonable
chance it's only exposed by normal thread jumps in VRP2 and since we
don't iterate, it's left in the IL. I haven't analyzed it in detail,
but my hope is that when I pull the backwards threading out into its own
pass that we'll start picking up more of these secondary effects.
gcc/ChangeLog:
* tree-ssa-threadedge.c (simplify_control_stmt_condition): Split
out into ...
(simplify_control_stmt_condition_1): ... here. Recurse into
BIT_AND_EXPRs and BIT_IOR_EXPRs.
gcc/testsuite/ChangeLog:
* gcc.dg/tree-ssa/ssa-thread-14.c: New test.
---
I fixed a few formatting nits (too long lines).
+
+ if (res1 != NULL_TREE && res2 != NULL_TREE)
+ {
+ if (rhs_code == BIT_AND_EXPR
+ && TYPE_PRECISION (TREE_TYPE (op0)) == 1
+ && integer_nonzerop (res1)
+ && integer_nonzerop (res2))
+ {
+ /* If A != 0 and B != 0 then (bool)(A | B) != 0 is true. */
+ if (cond_code == NE_EXPR)
+ return one_cst;
+ /* If A != 0 and B != 0 then (bool)(A | B) == 0 is false. */
+ if (cond_code == EQ_EXPR)
+ return zero_cst;
I think you wanted (A & B) in the two immediately preceding comments,
which I fixed.
I suspect there's other stuff we could do in this space, but you've
probably covered the most important cases.
+ /* Handle (A CMP B) CMP 0. */
+ else if (TREE_CODE_CLASS (gimple_assign_rhs_code (def_stmt))
+ == tcc_comparison)
+ {
+ tree rhs1 = gimple_assign_rhs1 (def_stmt);
+ tree rhs2 = gimple_assign_rhs2 (def_stmt);
+
+ tree_code new_cond = gimple_assign_rhs_code (def_stmt);
+ if (cond_code == EQ_EXPR)
+ new_cond = invert_tree_comparison (new_cond, false);
+
+ tree res
+ = simplify_control_stmt_condition_1 (e, def_stmt, avail_exprs_stack,
+ rhs1, new_cond, rhs2,
+ dummy_cond, simplify,
+ handle_dominating_asserts,
+ limit - 1);
+ if (res != NULL_TREE && is_gimple_min_invariant (res))
+ return res;
I was a bit confused by this case, but then realized that we already
narrowed COND_CODE to EQ_EXPR/NE_EXPR.
I made the minor edits noted above and committed the change.
Thanks,
Jeff
