From patchwork Sun Jul 2 22:49:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 783267 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3x157N18GKz9s5L for ; Mon, 3 Jul 2017 08:49:50 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="JbyRRail"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:references:subject:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=CWslHi+rbEQZuDco2 VyMYdBjODVoqmlAqXBFz3xp/ldjvZOe+0Yz3p9p+4hE84jAABDVAKBNactAcAMSn y/rLKLAIGlXe76szMSMeDxmFcfUQ/so+ZvCtqNk+tiLIuwAVLoxN/VdRT0SaB79P zNuhNis5d9WEPfh1wZcggcw2/w= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:references:subject:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=IiOcEpmI/uG8U5Jdp/j4Zdm TZfI=; b=JbyRRailtg0KK3lYunPfi7I4txKKVJPlvuavc+4Errzovrmstrd9sU2 xGLrJrltvMS8EBVFY2Um+DEnYfz8zHhnke+THMu3KOYUTVa+61b/rW3iU5d7y0s1 6FQeh8fwxJ6Jg9vfv+oKvzFdCiGq4nq6VYetMj6XqCX+SVkxJVno= Received: (qmail 101577 invoked by alias); 2 Jul 2017 22:49:42 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 101559 invoked by uid 89); 2 Jul 2017 22:49:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS, URIBL_RED autolearn=ham version=3.3.2 spammy= X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 02 Jul 2017 22:49:40 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-MBX-04.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1dRngQ-00013C-26 from Tom_deVries@mentor.com ; Sun, 02 Jul 2017 15:49:38 -0700 Received: from [127.0.0.1] (137.202.0.87) by SVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4) with Microsoft SMTP Server (TLS) id 15.0.1263.5; Sun, 2 Jul 2017 23:49:34 +0100 From: Tom de Vries To: Richard Biener CC: GCC Patches References: Subject: [PATCH, PR81192] Don't tail-merge blocks from different loops Message-ID: Date: Mon, 3 Jul 2017 00:49:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: X-ClientProxiedBy: svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) To SVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4) [ 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 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: duplicate of " 1 "pre" } } */ +/* { dg-final { scan-tree-dump-not "(?n)find_duplicates: duplicate of " "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)