On 03/18/14 04:13, bin.cheng wrote:
Hi,
After control flow graph change made by
http://gcc.gnu.org/ml/gcc-patches/2014-02/msg01492.html, case
gcc.dg/tree-ssa/ssa-dom-thread-4.c is broken on logical_op_short_circuit
targets including cortex-m3/cortex-m0.
The regression reveals a missed opportunity in jump threading, which causes
a forward basic block doesn't get removed in cfgcleanup after jump threading
in VRP1. Root cause is stated at the corresponding PR:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60363, please refer to it for
detailed report.
This patch fixes the issue by adding constant value instead of ssa_name as
the new phi argument. Bootstrap and test on x86_64, also test on cortex-m3
and the regression is gone.
I think this should wait for stage1, but would like to hear some comments
now. So does it look reasonable?
2014-03-18 Bin Cheng<bin.ch...@arm.com>
PR regression/60363
* gcc/tree-ssa-threadupdate.c (get_value_locus_in_path): New.
(copy_phi_args): New parameters. Call get_value_locus_in_path.
(update_destination_phis): New parameter.
(create_edge_and_update_destination_phis): Ditto.
(ssa_fix_duplicate_block_edges): Pass new arguments.
(thread_single_edge): Ditto.
This is a good and interesting catch. DOM knows how to propagate these
context sensitive equivalences which should expose the optimizable
forwarder blocks.
But I'm a big believer in catching as many CFG simplifications as early
as we can as they tend to have nice cascading effects. So if we can
pick it up by being smarter in how we duplicate arguments, then I'm all
for it.
+ for (int j = idx - 1; j >= 0; j--)
+ {
+ edge e = (*path)[j]->e;
+ if (e->dest == def_bb)
+ {
+ arg = gimple_phi_arg_def (def_phi, e->dest_idx);
+ *locus = gimple_phi_arg_location (def_phi, e->dest_idx);
+ return (TREE_CODE (arg) == INTEGER_CST ? arg : def);
Presumably any constant that can legitimately appear in a PHI node is
good here. So for example ADDR_EXPR <something in static storage> ought
to be handled as well.
One could also argue that we should go ahead and do a context sensitive
copy propagation here too if ARG turns out to be an SSA_NAME. You have
to be a bit more careful with those and use may_propagate_copy_p and
you'd probably want to test the loop depth of the SSA_NAMEs to ensure
you're not doing a propagation that is going to muck up LICM. See
loop_depth_of_name uses in tree-ssa-dom.c.
Overall I think it's good. We just need to resolve whether or not we
want to catch constant ADDR_EXPRs and/or do the context sensitive copy
propagations.
jeff
