From patchwork Fri Jul 6 16:36:49 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: 169492 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 622172C01FA for ; Sat, 7 Jul 2012 02:37:22 +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=1342197443; 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=WUwupp5WS1lnavrUHqxRF2ckFF8=; b=egRVhWl67uHEFS2B+yzCCcR4Ck7DiluDtRDiN4fT/8egjiP85aKzUgO3oGPklC YHs5vN79WBpY2deutX9OQhJ5ukCnW5BXVFoQD7b0QegbyfMBbO3OYEl05RexHIH9 Kqmk7uAENylmJkHmHDTQDpCv9u4VszNKdkI9Y3jzs0cCM= 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=M3w4n9QEorCEvAjCxBbmqEtwISTlRiGRaWFA0nsgeyOMBtIzrs9mQaGqjk92eb JH09YrwbEGd1mfTD10CTJt2kKWdphY75bigexDtFtJ4/r3yFjXnq/p23DFBcmqaH 9x8aYbRjhOuaEf9xTecLQGuK1PQuOfxdwbZp4hMfDCRG0=; Received: (qmail 31665 invoked by alias); 6 Jul 2012 16:37:18 -0000 Received: (qmail 31175 invoked by uid 22791); 6 Jul 2012 16:37:17 -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; Fri, 06 Jul 2012 16:37:03 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1SnBWc-00056f-Mg from Tom_deVries@mentor.com ; Fri, 06 Jul 2012 09:36:58 -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); Fri, 6 Jul 2012 09:36:58 -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; Fri, 6 Jul 2012 17:36:55 +0100 Message-ID: <4FF71421.3050704@mentor.com> Date: Fri, 6 Jul 2012 18:36:49 +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: Richard Guenther CC: Michael Matz , Ulrich Weigand , , , Andrew Pinski , Steven Bosscher Subject: Re: Tree tail merging breaks __builtin_unreachable optimization References: <201207041702.q64H2GGj017517@d06av02.portsmouth.uk.ibm.com> <4FF58D3E.4060105@mentor.com> <4FF5E0D1.30402@mentor.com> In-Reply-To: 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 06/07/12 13:01, Richard Guenther wrote: > On Thu, Jul 5, 2012 at 8:45 PM, Tom de Vries wrote: >> On 05/07/12 15:30, Michael Matz wrote: >>> Hi, >>> >>> On Thu, 5 Jul 2012, Tom de Vries wrote: >>> >>>> 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. >>> >>> Nope. That would require some more checks, in particular that the BB >>> containing builtin_unreachable doesn't contain any other side-effects. >>> Given this: >>> >>> if (i < 0) >>> { do_something_interesting(); >>> __builtin_unreachable(); >>> } >>> >>> moving the assert before the if would remove the if condition, hence >>> the call to do_something_interesting. You need to retain side-effects if >>> there are any. >>> >> >> Michael, >> >> Thanks for pointing that out. >> >> I tried a first stab at your suggestion of implementing the optimization in >> pass_fold_builtins, it works for the test-case. > > +static bool > +optimize_unreachable (gimple_stmt_iterator i) > +{ > + gimple stmt; > + basic_block bb; > + edge_iterator ei; > + edge e; > + > + for (gsi_prev (&i); !gsi_end_p (i); gsi_prev (&i)) > + { > + stmt = gsi_stmt (i); > + if (gimple_has_side_effects (stmt)) > + return false; > + } > > I think we should rely on DCE to remove stmts without side-effects before > __builtin_unreachable. Thus I'd make this > > basic_block bb = gsi_bb (i); > for (gsi = gsi_start (bb); !gsi_end_p (i); gsi_next (&gsi)) > { > if (is_gimple_debug ()) > continue; > if (gimple_code () == GIMPLE_LABEL) > /* Verify we do not need to preserve the label. */; > if (gsi_stmt () != gsi_stmt (i)) > return false; > } > > ... > > thus simply require the builtin be the first statement in the block. Done. > > As for the label I'm concerned about > > void foo (int b, int c) > { > void *x = &&lab; > if (b) > { > lab: > __builtin_unreachable (); > } > lab2: > if (c) > x = &&lab2; > goto *x; > } > > non-sensical, of course, but "valid". > Added this example as test-case. Bootstrapped and reg-tested (ada inclusive) on x86_64. OK for trunk? Thanks, - Tom 2012-07-06 Tom de Vries Richard Guenther * tree-ssa-ccp.c (optimize_unreachable): New function. (execute_fold_all_builtins): Use optimize_unreachable to optimize BUILT_IN_UNREACHABLE. Don't optimize after BUILT_IN_UNREACHABLE. * gcc.dg/builtin-unreachable-6.c: New test. * gcc.dg/builtin-unreachable-5.c: New test. Index: gcc/tree-ssa-ccp.c =================================================================== --- gcc/tree-ssa-ccp.c (revision 189007) +++ gcc/tree-ssa-ccp.c (working copy) @@ -2318,6 +2318,69 @@ optimize_stdarg_builtin (gimple call) } } +/* Attemp to make the block of __builtin_unreachable I unreachable by changing + the incoming jumps. Return true if at least one jump was changed. */ + +static bool +optimize_unreachable (gimple_stmt_iterator i) +{ + basic_block bb = gsi_bb (i); + gimple_stmt_iterator gsi; + gimple stmt; + edge_iterator ei; + edge e; + bool ret; + + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) + { + stmt = gsi_stmt (gsi); + + if (is_gimple_debug (stmt)) + continue; + + if (gimple_code (stmt) == GIMPLE_LABEL) + { + /* Verify we do not need to preserve the label. */ + if (FORCED_LABEL (gimple_label_label (stmt))) + return false; + + continue; + } + + /* Only handle the case that __builtin_unreachable is the first statement + in the block. We rely on DCE to remove stmts without side-effects + before __builtin_unreachable. */ + if (gsi_stmt (gsi) != gsi_stmt (i)) + return false; + } + + ret = false; + FOR_EACH_EDGE (e, ei, bb->preds) + { + gsi = gsi_last_bb (e->src); + stmt = gsi_stmt (gsi); + + if (stmt && gimple_code (stmt) == GIMPLE_COND) + { + if (e->flags & EDGE_TRUE_VALUE) + gimple_cond_make_false (stmt); + else if (e->flags & EDGE_FALSE_VALUE) + gimple_cond_make_true (stmt); + else + gcc_unreachable (); + } + else + { + /* Todo: handle other cases, f.i. switch statement. */ + continue; + } + + ret = true; + } + + return ret; +} + /* A simple pass that attempts to fold all builtin functions. This pass is run after we've propagated as many constants as we can. */ @@ -2379,6 +2442,11 @@ execute_fold_all_builtins (void) gsi_next (&i); continue; + case BUILT_IN_UNREACHABLE: + if (optimize_unreachable (i)) + cfg_changed = true; + break; + case BUILT_IN_VA_START: case BUILT_IN_VA_END: case BUILT_IN_VA_COPY: @@ -2393,6 +2461,9 @@ execute_fold_all_builtins (void) continue; } + if (result == NULL_TREE) + break; + if (dump_file && (dump_flags & TDF_DETAILS)) { fprintf (dump_file, "Simplified\n "); Index: gcc/testsuite/gcc.dg/builtin-unreachable-6.c =================================================================== --- /dev/null (new file) +++ gcc/testsuite/gcc.dg/builtin-unreachable-6.c (revision 0) @@ -0,0 +1,21 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-fab" } */ + +void +foo (int b, int c) +{ + void *x = &&lab; + if (b) + { +lab: + __builtin_unreachable (); + } +lab2: + if (c) + x = &&lab2; + goto *x; +} + +/* { dg-final { scan-tree-dump-times "lab:" 1 "fab" } } */ +/* { dg-final { scan-tree-dump-times "__builtin_unreachable" 1 "fab" } } */ +/* { dg-final { cleanup-tree-dump "fab" } } */ Index: gcc/testsuite/gcc.dg/builtin-unreachable-5.c =================================================================== --- /dev/null (new file) +++ gcc/testsuite/gcc.dg/builtin-unreachable-5.c (revision 0) @@ -0,0 +1,23 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-fab" } */ + +int +foo (int a) +{ + if (a <= 0) + { + L1: + __builtin_unreachable (); + } + + if (a > 2) + goto L1; + + return a > 0; +} + +/* { dg-final { scan-tree-dump-times "if \\(" 0 "fab" } } */ +/* { dg-final { scan-tree-dump-times "goto" 0 "fab" } } */ +/* { dg-final { scan-tree-dump-times "L1:" 0 "fab" } } */ +/* { dg-final { scan-tree-dump-times "__builtin_unreachable" 0 "fab" } } */ +/* { dg-final { cleanup-tree-dump "fab" } } */