From patchwork Thu Jul 5 12:49:02 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 169150 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]) by ozlabs.org (Postfix) with SMTP id 639652C01FF for ; Thu, 5 Jul 2012 22:49:31 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1342097371; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC: Subject:References:In-Reply-To:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=++vfhC7efC9nIEOgQ6NAanSJlJs=; b=mGSnZ0bmKD+dMPB97R/uFFgxFUOb4LNgybBsG9h3RXQWSc1JYvWuqwojOC5iUN OLjAL3tGdvjHF7rYfoBvm0ZFGBIj0QxD3b5t4wPa4wM3DRZfCGrGFR568DQBcmzN ywSi1agAqsQWRCOdHgE4auSFe+9UNAx5IAKzoIeuqQSV4= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=uGcNgfloX/S0aql17vb/W4IHu8fJCganFJedzCk4IuwSBM30LAAHmPCFpEEd+e ZXFEu5TPSWHqU/q9dz4FhlkI4rbOhKHV9O6afXv/BSdWhE2bsufD/S0m6Bmhk/P7 eODyRcMfnoxX6etK/2iAmIqRPdotmP2YXF/1Kps6UDKrs=; Received: (qmail 31756 invoked by alias); 5 Jul 2012 12:49:27 -0000 Received: (qmail 31748 invoked by uid 22791); 5 Jul 2012 12:49:26 -0000 X-SWARE-Spam-Status: No, hits=-4.2 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, TW_CF, TW_TM X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 05 Jul 2012 12:49:12 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1SmlUc-000724-GO from Tom_deVries@mentor.com ; Thu, 05 Jul 2012 05:49:10 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Thu, 5 Jul 2012 05:49:10 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Thu, 5 Jul 2012 13:49:07 +0100 Message-ID: <4FF58D3E.4060105@mentor.com> Date: Thu, 5 Jul 2012 14:49:02 +0200 From: Tom de Vries User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120615 Thunderbird/13.0.1 MIME-Version: 1.0 To: Ulrich Weigand CC: , , Andrew Pinski , Steven Bosscher , Richard Guenther , Michael Matz Subject: Re: Tree tail merging breaks __builtin_unreachable optimization References: <201207041702.q64H2GGj017517@d06av02.portsmouth.uk.ibm.com> In-Reply-To: <201207041702.q64H2GGj017517@d06av02.portsmouth.uk.ibm.com> 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 On 04/07/12 19:02, Ulrich Weigand wrote: > Any suggestions how to fix this? Should tail merging detect > __builtin_unreachable and not merge such block? Or else, should > the CFG optimizer be extended (how?) to handle unreachable blocks > with multiple predecessors better? Ulrich, I extended the example as such: ... int foo(int a) { if (a <= 0) { #ifdef MERGED L1: #endif __builtin_unreachable(); } if (a > 2) #ifdef MERGED goto L1; #else __builtin_unreachable(); #endif return a > 0; } ... Indeed I can reproduce the problem: ... $ gcc unreachable.c -O2 -S -o- -ftree-tail-merge foo: .cfi_startproc testl %edi, %edi jle .L3 cmpl $2, %edi jg .L3 movl $1, %eax ret .L3: .cfi_endproc ... And I can make the problem go away with -fno-tree-tail-merge: ... $ gcc unreachable.c -O2 -S -o- -fno-tree-tail-merge foo: .cfi_startproc movl $1, %eax ret .cfi_endproc ... But I can also reproduce the problem with -fno-tree-tail-merge by using -DMERGED: ,,, $ gcc unreachable.c -O2 -S -o- -fno-tree-tail-merge -DMERGED foo: .cfi_startproc testl %edi, %edi jle .L3 cmpl $2, %edi jg .L3 movl $1, %eax ret .L3: .cfi_endproc ,,, So it seems that this is a problem of a missed optimization, triggered by, but not exclusively triggered by -ftree-tail-merge. How to fix the missed optimization, I'm not sure where it should be done. I think the optimization could be done in tree-vrp. Currently the assert expressions look like this: ... .foo (intD.6 aD.1711) { _BoolD.1693 D.1720; intD.6 D.1719; # BLOCK 2 freq:10000 # PRED: ENTRY [100.0%] (fallthru,exec) if (aD.1711_1(D) <= 0) goto ; else goto ; # SUCC: 3 [0.0%] (true,exec) 4 [100.0%] (false,exec) # BLOCK 3 freq:4 # PRED: 2 [0.0%] (true,exec) # VUSE <.MEMD.1722_4(D)> # USE = nonlocal # CLB = nonlocal __builtin_unreachableD.997 (); # SUCC: # BLOCK 4 freq:9996 # PRED: 2 [100.0%] (false,exec) aD.1711_6 = ASSERT_EXPR 0>; if (aD.1711_6 > 2) goto ; else goto ; # SUCC: 5 [0.0%] (true,exec) 6 [100.0%] (false,exec) # BLOCK 5 freq:4 # PRED: 4 [0.0%] (true,exec) # VUSE <.MEMD.1722_4(D)> # USE = nonlocal # CLB = nonlocal __builtin_unreachableD.997 (); # SUCC: # BLOCK 6 freq:9992 # PRED: 4 [100.0%] (false,exec) aD.1711_5 = ASSERT_EXPR ; D.1720_2 = aD.1711_5 > 0; D.1719_3 = (intD.6) D.1720_2; # VUSE <.MEMD.1722_4(D)> return D.1719_3; # SUCC: EXIT [100.0%] } .. The asserts allow the return result to be optimized, but not the cfg conditions. AFAIU, we can insert the asserts earlier. F.i., we can insert aD.1711_6 = ASSERT_EXPR 0> before the GIMPLE_COND in bb2. Attached proof-of-concept patch implements this, and works for this example both with and without -DMERGED, independent of -ftree-tail-merge. The only problem I can see with this approach is that doing this early (vrp1) basically removes range annotations which might have had an effect later on. It could be and idea to only do this in vrp2, which is not much earlier than expand, were the rtl cfg optimization kicks in for the first time. Thanks, - Tom Index: gcc/tree-vrp.c =================================================================== --- gcc/tree-vrp.c (revision 189007) +++ gcc/tree-vrp.c (working copy) @@ -4222,9 +4222,11 @@ register_new_assert_for (tree name, tree gcc_checking_assert (bb == NULL || e == NULL); +#if 0 if (e == NULL) gcc_checking_assert (gimple_code (gsi_stmt (si)) != GIMPLE_COND && gimple_code (gsi_stmt (si)) != GIMPLE_SWITCH); +#endif /* Never build an assert comparing against an integer constant with TREE_OVERFLOW set. This confuses our undefined overflow warning @@ -4449,7 +4451,28 @@ register_edge_assert_for_2 (tree name, e if (live_on_edge (e, name) && !has_single_use (name)) { - register_new_assert_for (name, name, comp_code, val, NULL, e, bsi); + bool only_reachable_edge = true; + edge_iterator ei2; + edge e2; + FOR_EACH_EDGE (e2, ei2, e->src->succs) + { + if (e2 == e) + continue; + if (EDGE_COUNT (e2->dest->succs) == 0) + continue; + only_reachable_edge = false; + break; + } + + if (only_reachable_edge) + { + gimple_stmt_iterator bsi2 = bsi; + gsi_prev (&bsi2); + register_new_assert_for (name, name, comp_code, val, e->src, NULL, bsi2); + return true; + } + else + register_new_assert_for (name, name, comp_code, val, NULL, e, bsi); retval = true; } @@ -5569,7 +5592,13 @@ process_assert_insertions_for (tree name /* Otherwise, we can insert right after LOC->SI iff the statement must not be the last statement in the block. */ stmt = gsi_stmt (loc->si); - if (!stmt_ends_bb_p (stmt)) + if (stmt == NULL) + { + gimple_stmt_iterator si2 = gsi_last_bb (gsi_bb (loc->si)); + gsi_insert_before (&si2, assert_stmt, GSI_SAME_STMT); + return false; + } + else if (!stmt_ends_bb_p (stmt)) { gsi_insert_after (&loc->si, assert_stmt, GSI_SAME_STMT); return false;