On Mon, 31 Aug 2015, Tom de Vries wrote: > On 23/07/15 15:44, Richard Biener wrote: > > On Mon, 20 Jul 2015, Tom de Vries wrote: > > > > > On 09/07/15 13:04, Richard Biener wrote: > > > > On Thu, 9 Jul 2015, Tom de Vries wrote: > > > > > > > > > On 07/07/15 17:58, Tom de Vries wrote: > > > > > > > If you can > > > > > > > handle one exit edge I also can't see the difficulty in handling > > > > > > > all exit edges. > > > > > > > > > > > > > > > > > > > Agreed, that doesn't look to complicated. I could call > > > > > > rewrite_virtuals_into_loop_closed_ssa for all loops in > > > > > > rewrite_virtuals_into_loop_closed_ssa, to get non-single_dom_exit > > > > > > loops > > > > > > exercising the code, and fix what breaks. > > > > > > > > > > Hmm, I just realised, it's more complicated than I thought. > > > > > > > > > > In loops with single_dom_exit, the exit dominates the uses outside the > > > > > loop, > > > > > so I can replace the uses of the def with the uses of the exit phi > > > > > result. > > > > > > > > > > If !single_dom_exit, the exit(s) may not dominate all uses, and I need > > > > > to > > > > > insert non-loop-exit phi nodes to deal with that. > > > > > > > > Yes. This is why I originally suggested to amend the regular > > > > loop-close-SSA rewriting code. > > > > > > > > > > This patch renames rewrite_into_loop_closed_ssa to > > > rewrite_into_loop_closed_ssa_1, and adds arguments: > > > - a loop argument, to limit the defs for which the uses are > > > rewritten > > > - a use_flags argument, to determine the type of uses rewritten: > > > SSA_OP_USE/SSA_OP_VIRTUAL_USES/SSA_OP_ALL_USES > > > > > > The original rewrite_into_loop_closed_ssa is reimplemented using > > > rewrite_into_loop_closed_ssa_1. > > > > > > And the !single_dom_exit case of rewrite_into_loop_closed_ssa is > > > implemented > > > using rewrite_into_loop_closed_ssa_1. [ The patch was tested as attached, > > > always using rewrite_into_loop_closed_ssa_1, otherwise it would not be > > > triggered. ] > > > > > > Bootstrapped and reg-tested on x86_64. > > > > > > Is this sort of what you had in mind? > > > > Yes. New functions need a comment > > Done. > > > and instead of iterating over > > all function BBs and checking bb->loop_father please use > > get_loop_body (). > > > > Done. > > > Of course in the final version #if 0 stuff shouldn't remain. > > Done. > > > What's > > the cost difference of removing the single_dom_exit special-case? > > > > I'm not sure. But, in order to avoid having unexercised code, I removed the > single_dom_exit special-case in this patch. I think the code is not called > often enough to have an impact in speed. And if we want the single_dom_exit > special case, we should probably add it generically, rather than having it > just for virtuals as it is done now. > > Bootstrapped and reg-tested on x86_64. > > OK for trunk?
Ok. Thanks, Richard.
