[PATCH, PR81192] Don't tail-merge blocks from different loops

[Tom de Vries <Tom_deVries at mentor dot com>]

[ 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.

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?

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)