On Wed, Aug 13, 2014 at 4:54 AM, Bin.Cheng <amker.ch...@gmail.com> wrote: > On Wed, Aug 13, 2014 at 4:40 AM, Jeff Law <l...@redhat.com> wrote: >> On 08/12/14 14:23, Richard Biener wrote: >>> >>> On August 12, 2014 8:31:16 PM CEST, Jeff Law <l...@redhat.com> wrote: >>>> >>>> On 08/12/14 11:46, Steve Ellcey wrote: >>>>> >>>>> After talking to Jeff Law at the GCC Cauldron I have updated my >>>> >>>> switch >>>>> >>>>> shortcut plugin pass to try and address this optimization in the >>>> >>>> hopes of >>>>> >>>>> getting it added to GCC as a static pass. I fixed the code to build >>>> >>>> with >>>>> >>>>> the various C++ changes that have been happening in GCC but the >>>> >>>> current >>>>> >>>>> version I have included in this email is not working yet. When I run >>>> >>>> it >>>>> >>>>> on coremark I get errors like: >>>>> >>>>> core_state.c: In function 'core_bench_state': >>>>> core_state.c:43:8: error: size of loop 16 should be 13, not 5 >>>>> ee_u16 core_bench_state(ee_u32 blksize, ee_u8 *memblock, >>>>> ^ >>>>> core_state.c:43:8: error: bb 15 does not belong to loop 16 >>>>> core_state.c:43:8: error: bb 113 does not belong to loop 16 >>>>> core_state.c:43:8: error: bb 118 does not belong to loop 16 >>>>> core_state.c:43:8: error: bb 117 does not belong to loop 16 >>>>> (etc) >>>>> >>>>> Apparently there have been some changes to the loop information that >>>>> is built since GCC 4.9. I had hoped that adding a call to >>>> >>>> fix_loop_structure >>>>> >>>>> after recalculating the dominance information would fix this but it >>>> >>>> didn't. >>>>> >>>>> >>>>> Does anyone have any ideas on how to fix up the loop information that >>>> >>>> my >>>>> >>>>> optimization has messed as it duplicates blocks? I tried adding a >>>> >>>> call >>>>> >>>>> 'loop_optimizer_init (LOOPS_NORMAL)' before the fix_loop_structure >>>> >>>> but that >>>>> >>>>> did not seem to have any affect. >>>> >>>> Try setting the header & latch fields for the loop structure to NULL, >>>> then call loops_set_state (LOOPS_NEED_FIXUP). >>> >>> >>> But that is _not_ the appropriate way of keeping loops preserved! >> >> I think that's done when we've scrogged the loop beyond repair and want the >> structures rebuilt. IIRC, that's what you recommended we do. > > Sorry for disturbing with this patch irrelevant question. If I > understand correctly, we are now trying best to preserve loop > structure and LOOPS_NEED_FIXUP still works. My question is how we > decide when we can rebuild loop structure and when we need to preserve > it? If there is meta data stored in loop structure, does that mean it > should never be re-built?
Meta-data is associated with loops and ultimately loops are associated with their header basic-block. LOOPS_NEED_FIXUP tells the machinery to re-scan the body for loops but keep the existing header basic-block to loop structure association. The only case it does not do that is when you explicitely NULL a loop->header (you signal the loop is now no longer a loop) or when the discovery process discovers a loop which header basic-block didn't previously have a loop associated with or if a loops recorded header basic-block is no longer a loop header (by means of finding a latch edge going to it). I'd like to make both discovering a new loop and removing an existing loop (that wasn't marked for removal) an error. All that passes need to do is properly allocate a new loop structure for new loops headers and mark loops that no longer are loops properly. And of course do it "the right way". Note that the suspicious activity is logged in passes dump-files (well, in the pass performing the fix_loop_structure call) with TDF_DETAILS level as "fix_loop_structure: removing loop %d" and "flow_loops_find: discovered new loop %d with header %d". One issue with marking loops for removal is that we do that by NULL-ing its header which makes spurious removal detection impossible (as we no longer know its former header). We should fix that implementation detail. Richard. > > Thanks, > bin > >> >> jeff
