From patchwork Mon Jan 14 16:13:53 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 211838 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 17B0B2C0095 for ; Tue, 15 Jan 2013 03:14:51 +1100 (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=1358784892; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To:User-Agent: Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:Sender:Delivered-To; bh=wEPizvkO6jKO/woBpVYP dP+HrBA=; b=ZsN2osPgdqFr/tHfGH7RLKDb7kJCQMRLdpyWAdRTCB0PDk+ha++x S73HlOznd/7zBJ0lVwe83EtTACcjBqfDwr3GY4DbPOO/hnTZ+BS9RQuSE74UQh02 msXtsPeJf3j65T/fvSCJ0P+JIxhrSHcPOg/P0fqpB7MODUrC/i/MFbg= 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:Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:Content-Type:Content-Disposition:In-Reply-To:User-Agent:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=tyOyGOVkSKaV+aaYbKl7WqeHA/4xflwbObG6Ld05hmlvHPoPoLn2snI3XrHw6A myKpJdnpc314HHn5aTAU385wqAcoSv+smRxFX30cPaeOuWyIEk2zZTCTdoINNk7R 1wtuhBH3C2IcMmeCCusFi8NBmPskmIQvkgjEC18FU7x9w=; Received: (qmail 24131 invoked by alias); 14 Jan 2013 16:14:10 -0000 Received: (qmail 24079 invoked by uid 22791); 14 Jan 2013 16:14:09 -0000 X-SWARE-Spam-Status: No, hits=-6.4 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 14 Jan 2013 16:13:58 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r0EGDwqd006681 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 14 Jan 2013 11:13:58 -0500 Received: from redhat.com (ovpn-116-21.ams2.redhat.com [10.36.116.21]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r0EGDrm0027648 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Mon, 14 Jan 2013 11:13:56 -0500 Date: Mon, 14 Jan 2013 17:13:53 +0100 From: Marek Polacek To: Zdenek Dvorak Cc: Steven Bosscher , GCC Patches , Richard Guenther Subject: Re: [PATCH] Fix PR55833 + cheaper checking Message-ID: <20130114161353.GB5414@redhat.com> References: <20130110173143.GD20218@redhat.com> <20130110221943.GA14720@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130110221943.GA14720@kam.mff.cuni.cz> User-Agent: Mutt/1.5.20 (2009-06-14) 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 Thu, Jan 10, 2013 at 11:19:43PM +0100, Zdenek Dvorak wrote: > I agree -- at the very least, unswitch_single_loop should check whether there > is any possiblity it could have affected irreducible loops information (this > should only be the case when some already existing irreducible loop is altered > in the progress). Which is what it (or more precisely, remove_path function > used by it) tries to do -- so is should be sufficient to check why this fails > for the considered testcase, and make sure the situation is correctly detected, Actually, in this case, we don't call remove_path from unswitch_single_loop at all. So, here's another stab at it. In this version, we will call mark_irreducible_loops only in case where we're removing a loop from loop hierarchy tree. Because when we do that (and we're in some irreducible region), the edge that connects those two loops should be marked as EDGE_IRREDUCIBLE_LOOP. And the preheader BB eventually as BB_IRREDUCIBLE_LOOP. Does this look any better? I'm not actually checking whether we really are in a irreducible region, should that be done (how?)? Thanks. Bootstrap and regtest pending on x86_64-unknown-linux-gnu. 2013-01-14 Richard Biener Marek Polacek PR rtl-optimization/55833 * loop-unswitch.c (unswitch_loops): Move loop verification... (unswitch_single_loop): ...here. Call mark_irreducible_loops. * cfgloopmanip.c (fix_loop_placement): Add IRRED_INVALIDATED parameter. Set it to true when we're removing a loop from hierarchy tree. (fix_bb_placements): Adjust caller. (fix_loop_placements): Likewise. * gcc.dg/pr55833.c: New test. Marek --- gcc/loop-unswitch.c.mp 2013-01-10 16:50:28.899559875 +0100 +++ gcc/loop-unswitch.c 2013-01-14 15:44:30.574158810 +0100 @@ -145,12 +144,7 @@ unswitch_loops (void) /* Go through inner loops (only original ones). */ FOR_EACH_LOOP (li, loop, LI_ONLY_INNERMOST) - { - unswitch_single_loop (loop, NULL_RTX, 0); -#ifdef ENABLE_CHECKING - verify_loop_structure (); -#endif - } + unswitch_single_loop (loop, NULL_RTX, 0); iv_analysis_done (); } @@ -370,6 +364,10 @@ unswitch_single_loop (struct loop *loop, nloop = unswitch_loop (loop, bbs[i], copy_rtx_if_shared (cond), cinsn); gcc_assert (nloop); +#ifdef ENABLE_CHECKING + verify_loop_structure (); +#endif + /* Invoke itself on modified loops. */ unswitch_single_loop (nloop, rconds, num + 1); unswitch_single_loop (loop, conds, num + 1); --- gcc/cfgloopmanip.c.mp 2013-01-14 14:42:52.502002650 +0100 +++ gcc/cfgloopmanip.c 2013-01-14 16:48:40.693799547 +0100 @@ -111,10 +111,13 @@ fix_bb_placement (basic_block bb) /* Fix placement of LOOP inside loop tree, i.e. find the innermost superloop of LOOP to that leads at least one exit edge of LOOP, and set it as the immediate superloop of LOOP. Return true if the immediate superloop - of LOOP changed. */ + of LOOP changed. + + IRRED_INVALIDATED is set to true if a change in the loop structures might + invalidate the information about irreducible regions. */ static bool -fix_loop_placement (struct loop *loop) +fix_loop_placement (struct loop *loop, bool *irred_invalidated) { unsigned i; edge e; @@ -136,6 +139,9 @@ fix_loop_placement (struct loop *loop) flow_loop_tree_node_remove (loop); flow_loop_tree_node_add (father, loop); + /* We may need to recompute irreducible loops. */ + *irred_invalidated = true; + /* The exit edges of LOOP no longer exits its original immediate superloops; remove them from the appropriate exit lists. */ FOR_EACH_VEC_ELT (exits, i, e) @@ -212,7 +218,7 @@ fix_bb_placements (basic_block from, if (from->loop_father->header == from) { /* Subloop header, maybe move the loop upward. */ - if (!fix_loop_placement (from->loop_father)) + if (!fix_loop_placement (from->loop_father, irred_invalidated)) continue; target_loop = loop_outer (from->loop_father); } @@ -965,7 +971,7 @@ fix_loop_placements (struct loop *loop, while (loop_outer (loop)) { outer = loop_outer (loop); - if (!fix_loop_placement (loop)) + if (!fix_loop_placement (loop, irred_invalidated)) break; /* Changing the placement of a loop in the loop tree may alter the --- gcc/testsuite/gcc.dg/pr55833.c.mp 2013-01-10 17:23:26.016102692 +0100 +++ gcc/testsuite/gcc.dg/pr55833.c 2013-01-10 17:23:15.898073384 +0100 @@ -0,0 +1,28 @@ +/* PR rtl-optimization/55833 */ +/* { dg-do compile } */ +/* { dg-options "-O3" } */ + +int a, b, c; + +void foo() +{ + unsigned d, l, *p, k = 1; + + if(bar()) + { +label: + if((a = a <= 0)) + { + if(c) + d = b; + + if (b || d ? l : k ? : 0) + a = d = 0; + + goto label; + } + } + + while(*p++) + goto label; +}