On Thu, May 16, 2013 at 5:38 AM, Jeff Law <l...@redhat.com> wrote: > On 05/15/2013 12:28 PM, Steve Ellcey wrote: >> >> Here is a patch that adds a flag to gimple_duplicate_sese_region to tell >> it whether or not to update the dominator information. I had to add the >> same flag to copy_bbs to make it all work. How does this look? I >> tested it with a bootstrap and test on x86 (with my optimization >> enabled) and got no regressions. >> >> 2013-05-15 Steve Ellcey <sell...@imgtec.com> >> >> * cfghooks.c (copy_bbs): Add update_dominance argument. >> * cfghooks.h (copy_bbs): Update prototype. >> * tree-cfg.c (gimple_duplicate_sese_region): >> Add update_dominance argument. >> * tree-flow.h (gimple_duplicate_sese_region): Update prototype. >> * tree-ssa-loop-ch.c (copy_loop_headers): Update >> gimple_duplicate_sese_region call. >> * tree-vect-loop-manip.c (slpeel_tree_duplicate_loop_to_edge_cfg): >> Update copy_bbs call. >> * cfgloopmanip.c (duplicate_loop_to_header_edge): Ditto. >> * trans-mem.c (ipa_uninstrument_transaction): Ditto. > > So I'd change gimple_duplicate_sese_region to gimple_duplicate_seme region > per comments from others. > > Where you document UPDATE_DOMINANCE, I'd add something like: When > UPDATE_DOMINANCE is true, it is assumed that duplicating the region (or > copying the blocks for copy_bbs) may change the dominator tree in ways that > are not suitable for an incremental update and the caller is responsible for > destroying and recomputing the dominator tree. > > Hmm, not terribly happy with that wording, but that gives you an idea of > what I'm after. When would someone set UPDATE_DOMINANCE to true and what > are their responsibilities when they do so. > > Approved with the name change and a better comment for UPDATE_DOMINANCE.
Btw, the function does _not_ handle arbitrary SEME regions - it only handles a single exit correctly and assumes no (SSA) data flows across the others. So I'd rather not rename it. Richard. > Jeff >
