On Mon, Feb 24, 2014 at 9:12 PM, 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? > > > 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. >
This change: if (dest->loop_father->header == dest) - return false; + { + if (loops_state_satisfies_p (LOOPS_HAVE_PREHEADERS) + && bb->loop_father->header != dest) + return false; + + if (loops_state_satisfies_p (LOOPS_HAVE_SIMPLE_LATCHES) + && bb->loop_father->header == dest) + return false; + } } miscompiled 435.gromacs in SPEC CPU 2006 on x32 with -O3 -funroll-loops -ffast-math -fwhole-program -flto=jobserver -fuse-linker-plugin This patch changes loops without LOOPS_HAVE_PREHEADERS nor LOOPS_HAVE_SIMPLE_LATCHES from returning false to returning true. I don't have a small testcase. But this patch: diff --git a/gcc/tree-cfgcleanup.c b/gcc/tree-cfgcleanup.c index b5c384b..2ba673c 100644 --- a/gcc/tree-cfgcleanup.c +++ b/gcc/tree-cfgcleanup.c @@ -323,6 +323,10 @@ tree_forwarder_block_p (basic_block bb, bool phi_wanted) if (loops_state_satisfies_p (LOOPS_HAVE_SIMPLE_LATCHES) && bb->loop_father->header == dest) return false; + + if (!loops_state_satisfies_p (LOOPS_HAVE_PREHEADERS) + && !loops_state_satisfies_p (LOOPS_HAVE_SIMPLE_LATCHES)) + return false; } } fixes the regression. Does it make any senses? -- H.J.