On Tue, Feb 25, 2014 at 12:20 PM, bin.cheng <bin.ch...@arm.com> wrote: > Updated as comments.
Ok. Thanks, Richard. > Thanks, > bin > >> -----Original Message----- >> From: Richard Biener [mailto:richard.guent...@gmail.com] >> Sent: Tuesday, February 25, 2014 6:38 PM >> To: Bin Cheng >> Cc: GCC Patches >> Subject: Re: [PATCH GCC]Allow cfgcleanup to remove forwarder loop >> preheaders and latches >> >> On Tue, Feb 25, 2014 at 6:12 AM, bin.cheng <bin.ch...@arm.com> wrote: >> > Hi, >> > This patch is to fix regression reported in PR60280 by removing >> > forward loop headers/latches in cfg cleanup if possible. Several >> > tests are broken by this change since cfg cleanup is shared by all >> > optimizers. Some tests has already been fixed by recent patches, I went >> through and fixed the others. >> > One case needs to be clarified is "gcc.dg/tree-prof/update-loopch.c". >> > When GCC removing a basic block, it checks profile information by >> > calling check_bb_profile after redirecting incoming edges of the bb. >> > This certainly results in warnings about invalid profile information >> > and causes the case to fail. I will send a patch to skip checking >> > profile information for a removing basic block in stage 1 if it sounds >> > reasonable. For now I just twisted the case itself. >> > >> > Bootstrap and tested on x86_64 and arm_a15. >> > >> > Is it OK? >> >> Can you document the extra threading we do in pr21559.c? The comment >> still talks about two threadings we should perform. >> >> Also the ivopt_* adjustmens would be better done by matching >> "ivtmp.[0-9_]* = PHI" instead of matching ivtmp in one of the PHI > arguments. >> >> @@ -497,6 +507,9 @@ remove_forwarder_block (basic_block bb) >> set_immediate_dominator (CDI_DOMINATORS, dest, dom); >> } >> >> + if (current_loops && bb->loop_father->latch == bb) >> + bb->loop_father->latch = dest; >> + >> /* And kill the forwarder block. */ >> delete_basic_block (bb); >> >> can you add a comment here? I had >> >> @@ -497,7 +500,12 @@ remove_forwarder_block (basic_block bb) >> set_immediate_dominator (CDI_DOMINATORS, dest, dom); >> } >> >> - /* And kill the forwarder block. */ >> + /* And kill the forwarder block, but first adjust its parent loop >> + latch info as otherwise the cfg hook has a hard time not to >> + kill the loop. */ >> + if (current_loops >> + && bb->loop_father->latch == bb) >> + bb->loop_father->latch = dest; >> delete_basic_block (bb); >> >> return true; >> >> in my patch. >> >> Thanks, >> Richard. >> >> > >> > 2014-02-25 Bin Cheng <bin.ch...@arm.com> >> > >> > PR target/60280 >> > * tree-cfgcleanup.c (tree_forwarder_block_p): Protect loop >> > preheaders and latches only if requested. Fix latch if it >> > is removed. >> > * tree-ssa-dom.c (tree_ssa_dominator_optimize): Set >> > LOOPS_HAVE_PREHEADERS. >> > >> > gcc/testsuite/ChangeLog >> > 2014-02-25 Bin Cheng <bin.ch...@arm.com> >> > >> > PR target/60280 >> > * gnat.dg/renaming5.adb: Change to two expected gotos. >> > * gcc.dg/tree-ssa/pr21559.c: Change back to three expected >> > jump threads. >> > * gcc.dg/tree-prof/update-loopch.c: Check two "Invalid sum" >> > messages for removed basic block. >> > * gcc.dg/tree-ssa/ivopt_1.c: Fix unreliable scanning string. >> > * gcc.dg/tree-ssa/ivopt_2.c: Ditto. >> > * gcc.dg/tree-ssa/ivopt_3.c: Ditto. >> > * gcc.dg/tree-ssa/ivopt_4.c: Ditto.
