| Message ID | c886dba1-c509-2066-4171-1b7d84c8d53b@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 3 Jul 2017, Tom de Vries wrote: > [ was: Re: [PATCH, PR81192] Fix sigsegv in find_same_succ_bb ] > > On 07/03/2017 12:26 AM, Tom de Vries wrote: > > [ Trying again with before.svg instead of before.pdf ] > > > > Hi, > > > > consider this test-case: > > ... > > unsigned a; > > int b, c; > > > > static int > > fn1 (int p1, int p2) > > { > > return p1 > 2147483647 - p2 ? p1 : p1 + p2; > > } > > > > void > > fn2 (void) > > { > > int j; > > a = 30; > > for (; a;) > > for (; c; b = fn1 (j, 1)) > > ; > > } > > ... > > > > When compiling the test-case with -Os, just before tail-merge it looks as in > > before.svg. > > > > During tail-merge, it runs into a sigsegv. > > > > What happens is the following: > > - tail-merge decides to merge blocks 4 and 6, and removes block 6. > > As pointed out in the PR, blocks 4 and 6 belong to different loops, and > shouldn't be merged. > > There is a test 'bb->loop_father->latch == bb' in find_same_succ_bb that is > supposed to keep the two blocks from merging, but the test is not working for > this example because the latch field is not defined (because the loop has in > fact two latches). > > This patch prevents blocks from different loops to be merged by testing for > bb->loop_father->num equivalence in tail-merge. You can compare bb->loop_father directly. > It also removes the unreliable test for 'bb->loop_father->latch == bb' in > find_same_succ_bb. > > Bootstrapped and reg-tested on x86_64. > > OK for trunk and gcc-[567]-branch? Ok for trunk with the above change, as we do not have any wrong-code/ICE testcase that fails on branches I'd not backport this. Maybe to the GCC 7 branch but certainly not farther. Thanks, Richard. > Thanks, > - Tom > >
Don't tail-merge blocks from different loops 2017-06-30 Tom de Vries <tom@codesourcery.com> PR tree-optimization/81192 * tree-ssa-tail-merge.c (same_succ_hash): Use bb->loop_father->num in hash. (same_succ::equal): Don't find bbs to be equal if bb->loop_father->num differs. (find_same_succ_bb): Remove obsolete test on bb->loop_father->latch. * gcc.dg/pr81192.c: Update. --- gcc/testsuite/gcc.dg/pr81192.c | 2 +- gcc/tree-ssa-tail-merge.c | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/gcc/testsuite/gcc.dg/pr81192.c b/gcc/testsuite/gcc.dg/pr81192.c index 57eb478..8b3a77a 100644 --- a/gcc/testsuite/gcc.dg/pr81192.c +++ b/gcc/testsuite/gcc.dg/pr81192.c @@ -19,4 +19,4 @@ fn2 (void) ; } -/* { dg-final { scan-tree-dump-times "(?n)find_duplicates: <bb .*> duplicate of <bb .*>" 1 "pre" } } */ +/* { dg-final { scan-tree-dump-not "(?n)find_duplicates: <bb .*> duplicate of <bb .*>" "pre" } } */ diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c index bb8a308..0865e86 100644 --- a/gcc/tree-ssa-tail-merge.c +++ b/gcc/tree-ssa-tail-merge.c @@ -479,6 +479,8 @@ same_succ_hash (const same_succ *e) hstate.add_int (size); BB_SIZE (bb) = size; + hstate.add_int (bb->loop_father->num); + for (i = 0; i < e->succ_flags.length (); ++i) { flags = e->succ_flags[i]; @@ -568,6 +570,9 @@ same_succ::equal (const same_succ *e1, const same_succ *e2) if (BB_SIZE (bb1) != BB_SIZE (bb2)) return 0; + if (bb1->loop_father->num != bb2->loop_father->num) + return 0; + gsi1 = gsi_start_nondebug_bb (bb1); gsi2 = gsi_start_nondebug_bb (bb2); gsi_advance_fw_nondebug_nonlocal (&gsi1); @@ -695,15 +700,7 @@ find_same_succ_bb (basic_block bb, same_succ **same_p) edge_iterator ei; edge e; - if (bb == NULL - /* Be conservative with loop structure. It's not evident that this test - is sufficient. Before tail-merge, we've just called - loop_optimizer_finalize, and LOOPS_MAY_HAVE_MULTIPLE_LATCHES is now - set, so there's no guarantee that the loop->latch value is still valid. - But we assume that, since we've forced LOOPS_HAVE_SIMPLE_LATCHES at the - start of pre, we've kept that property intact throughout pre, and are - keeping it throughout tail-merge using this test. */ - || bb->loop_father->latch == bb) + if (bb == NULL) return; bitmap_set_bit (same->bbs, bb->index); FOR_EACH_EDGE (e, ei, bb->succs)