> Indeed somewhat simple-minded - when originally fixing a similar testcase
> (heh ...) I improved things by improving CFG cleanup to fold some more
> conditions by looking at SSA defs, that improved things a lot. I also
> thought the real fix should involve some scalar optimization on a
> sub-range of the CFG. That should be easiest when performing the copy in
> the first place - after all we keep copy tables and such for the purpose
> of update-SSA so we might as well create a lattice from PHI nodes we
> disassemble for use by copy_bb ...
I also thought of similar approaches, but couldn't really come up with
something satisfactory.
> On the patch itself - can you call the simple CCP before we call
> cleanup_tree_cfg () please?
That was the original implementation (in fact, in the original implementation
the simple CCP was conditionalized on canonicalize_loop_induction_variables,
but you broke it a few days ago by moving the call to update_ssa :-)
The problem is that, if it is moved to before cleanup_tree_cfg, then:
1) you have more basic blocks (15 vs 2 for the testcase),
2) you also need to do simple CCP for degenerate PHI nodes.
That being said, I can certainly save in a bitmap the loop fathers for which
canonicalize_loop_induction_variables unrolled a child and do the simple CPP
(augmented for degenerate PHI nodes) on them between the calls to update_ssa
and cleanup_tree_cfg.
> We might get rid of that weirdo SSA lookup
> there again then:
>
> static bool
> cleanup_control_expr_graph (basic_block bb, gimple_stmt_iterator gsi)
> {
> ...
> /* For conditions try harder and lookup single-argument
> PHI nodes. Only do so from the same basic-block though
> as other basic-blocks may be dead already. */
> if (TREE_CODE (lhs) == SSA_NAME
> && !name_registered_for_update_p (lhs))
I'll investigate.
> + FOR_EACH_IMM_USE_ON_STMT (use, iter)
> + propagate_value (use, gimple_assign_rhs1 (stmt));
> +
> + fold_stmt_inplace (&use_stmt_gsi);
> + update_stmt (use_stmt);
>
> Use SET_USE (use, rhs1) and cache gimple_assign_rhs1 somewhere.
>
> if (fold_stmt_inplace (&use_stmt_gsi))
> update_stmt (use_stmt);
OK, will adjust, thanks.
--
Eric Botcazou
