| Message ID | CAMe9rOrc41DiXbR1ThdvD-e09MDjxWjcQeKcfOv3wg7apRMVoQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Feb 28, 2016 at 9:48 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Sat, Feb 27, 2016 at 3:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Feb 25, 2016 at 11:50 PM, Jeff Law <law@redhat.com> wrote: >>> On 02/25/2016 03:00 AM, Richard Biener wrote: >>>> >>>> >>>> So I fail to see how only successor edges are relevant. Isn't the >>>> important >>>> case to catch whether we remove an edge marked EDGE_IRREDUCIBLE_LOOP? >>>> Even if the BB persists we might have exposed a new loop here. >>>> >>>> Note that it is not safe to look at {BB,EDGE}_IRREDUCIBLE_LOOP if the loop >>>> state does not have LOOPS_HAVE_MARKED_IRREDUCIBLE_REGIONS set >>>> (the flags may be stale or missing). So it might be that we can't rely on >>>> non-loop passes modifying the CFG to handle this optimistically. >>>> >>>> Thus, how about (my main point) moving this to remove_edge instead, like >>> >>> Yea. That works. The !loops_state_satisfies_p check will almost certainly >>> cause us to trigger loop cleanups more often, but I think it's the >>> right/safe thing to do to catch cases where we haven't go the >>> IRREDUCIBLE_LOOP flags set. >>> >>> >>> Bootstrapped and regression tested on x86_64-linux-gnu. OK for the trunk? >>> >>> Thanks, >>> Jeff >>> >>> >>> PR tree-optimization/69740 >>> * cfghooks.c (remove_edge): Request loop fixups if we delete >>> an edge that might turn an irreducible loop into a natural >>> loop. >>> >>> PR tree-optimization/69740 >>> * gcc.c-torture/compile/pr69740-1.c: New test. >>> * gcc.c-torture/compile/pr69740-2.c: New test. >>> >> >> This caused: >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69996 >> >> and may be other build failures in SPEC CPU 2006. >> > > I checked this patch into trunk and will backport it to GCC 5 branch. > This test doesn't fail and all SPEC CPU 2000/2006 benchmarks pass > on GCC 5 branch with the fix for PR 69740 applied. Is it possible that > the fix for PR 69740 exposed a latent bug on trunk? > This will also show up on GCC 5 if ENABLE_CHECKING is defined.
On 02/28/2016 02:11 PM, H.J. Lu wrote: > On Sun, Feb 28, 2016 at 9:48 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Sat, Feb 27, 2016 at 3:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Thu, Feb 25, 2016 at 11:50 PM, Jeff Law <law@redhat.com> wrote: >>>> On 02/25/2016 03:00 AM, Richard Biener wrote: >>>>> >>>>> >>>>> So I fail to see how only successor edges are relevant. Isn't the >>>>> important >>>>> case to catch whether we remove an edge marked EDGE_IRREDUCIBLE_LOOP? >>>>> Even if the BB persists we might have exposed a new loop here. >>>>> >>>>> Note that it is not safe to look at {BB,EDGE}_IRREDUCIBLE_LOOP if the loop >>>>> state does not have LOOPS_HAVE_MARKED_IRREDUCIBLE_REGIONS set >>>>> (the flags may be stale or missing). So it might be that we can't rely on >>>>> non-loop passes modifying the CFG to handle this optimistically. >>>>> >>>>> Thus, how about (my main point) moving this to remove_edge instead, like >>>> >>>> Yea. That works. The !loops_state_satisfies_p check will almost certainly >>>> cause us to trigger loop cleanups more often, but I think it's the >>>> right/safe thing to do to catch cases where we haven't go the >>>> IRREDUCIBLE_LOOP flags set. >>>> >>>> >>>> Bootstrapped and regression tested on x86_64-linux-gnu. OK for the trunk? >>>> >>>> Thanks, >>>> Jeff >>>> >>>> >>>> PR tree-optimization/69740 >>>> * cfghooks.c (remove_edge): Request loop fixups if we delete >>>> an edge that might turn an irreducible loop into a natural >>>> loop. >>>> >>>> PR tree-optimization/69740 >>>> * gcc.c-torture/compile/pr69740-1.c: New test. >>>> * gcc.c-torture/compile/pr69740-2.c: New test. >>>> >>> >>> This caused: >>> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69996 >>> >>> and may be other build failures in SPEC CPU 2006. >>> >> >> I checked this patch into trunk and will backport it to GCC 5 branch. >> This test doesn't fail and all SPEC CPU 2000/2006 benchmarks pass >> on GCC 5 branch with the fix for PR 69740 applied. Is it possible that >> the fix for PR 69740 exposed a latent bug on trunk? >> > > This will also show up on GCC 5 if ENABLE_CHECKING is defined. HJ. I was going to add tests for all the problems exposed by the remove_edge change in due time. Reverting the patch to address the regressions was done without a test simply to bring the trunk back to a reasonable state as quickly as possible. There are multiple paths through the optimizers that can trigger this issue and a single test is not the best way to go here. Jeff
Index: ChangeLog =================================================================== --- ChangeLog (revision 233791) +++ ChangeLog (working copy) @@ -1,3 +1,8 @@ +2016-02-28 H.J. Lu <hongjiu.lu@intel.com> + + PR tree-optimization/69989 + * gcc.dg/torture/pr69989.c: New test. + 2016-02-28 Eric Botcazou <ebotcazou@adacore.com> * gcc.target/i386/stack-realign-win.c: New test. Index: gcc.dg/torture/pr69989.c =================================================================== --- gcc.dg/torture/pr69989.c (nonexistent) +++ gcc.dg/torture/pr69989.c (working copy) @@ -0,0 +1,23 @@ +/* { dg-do compile } */ + +extern int a, b, d; +extern char c[]; +void +fn1 (void) +{ + for (;;) + { + if (b) + { +LABEL_T5T5T: + for (; d < a; d++) + c[d] = 6; + } + break; + } + if (a > 6) + { + a = 4; + goto LABEL_T5T5T; + } +}