From patchwork Thu Nov 29 16:55:53 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 202784 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 64A7C2C0040 for ; Fri, 30 Nov 2012 03:56:26 +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=1354812986; 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=X5AT0a4mNKl6QL6ZC1e+ 9B+kWqE=; b=HXIjIzlZtKrKgXhFYCmFNxiBu0muG18pXRCuL8rTVctPDHBdl8J6 RYH2nAPqPw2Eyxnp0/9dEcaJrvsFXA9TeQk5W0y5Tz8XPrA2Ju7jIF+F+KlnMCVN HQDnjJcAehImkkD6hei1WQJdW+gtmCRN344wdQuhZnIO8RLLT/2/vgE= 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=pCGKIOI3/zwOcdqo68EqjINSq35bYcs8OG5H8+qxQz6DJ6vDItkpccJttbkyB5 8qV/YWYL5utB0rxO7BvYMZzW9du7e2Jw5rNd4rV4aHt3mltYxuuzZNcvNicXZGTv nSQUmUA9DSQ+vMOh7n2DhP9ZhV+EPhdYy56G2G3zU7G00=; Received: (qmail 28322 invoked by alias); 29 Nov 2012 16:56:16 -0000 Received: (qmail 28297 invoked by uid 22791); 29 Nov 2012 16:56:14 -0000 X-SWARE-Spam-Status: No, hits=-6.6 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, 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; Thu, 29 Nov 2012 16:56:02 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qATGtvna030800 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 29 Nov 2012 11:55:58 -0500 Received: from redhat.com (ovpn-116-24.ams2.redhat.com [10.36.116.24]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qATGtrCp009347 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 29 Nov 2012 11:55:56 -0500 Date: Thu, 29 Nov 2012 17:55:53 +0100 From: Marek Polacek To: Steven Bosscher Cc: Richard Biener , Eric Botcazou , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Don't bypass blocks with multiple latch edges (PR middle-end/54838) Message-ID: <20121129165553.GE10621@redhat.com> References: <20121126142843.GH17362@redhat.com> <1544820.Re9E01eJrW@polaris> <20121128182457.GB26585@redhat.com> <20121129153852.GC10621@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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, Nov 29, 2012 at 04:50:19PM +0100, Steven Bosscher wrote: > > 2012-11-29 Marek Polacek <> > > > > PR middle-end/54838 > > * cprop.c (bypass_block): Set header and latch to NULL when > > BB has more than one latch edge. > > (n_latches): New variable. > > You don't have to mention a new local variable in the ChangeLog. Ok. > But FWIW, not all DFS back edges are latches. Maybe name it n_back_edges? Yeah, sure. > > @@ -1605,7 +1605,8 @@ bypass_block (basic_block bb, rtx setcc, > > && dest != EXIT_BLOCK_PTR) > > { > > if (current_loops != NULL > > - && e->src->loop_father->latch == e->src) > > + && (e->src->loop_father->latch == e->src > > + || n_latch_edges > 1)) > > { > > /* ??? Now we are creating (or may create) a loop > > with multiple entries. Simply mark it for > > It seems to me that this threading should just not happen. Creating > loops with multiple entries is something to be avoided because most > loop-based optimizations don't work on irreducible regions. So this > affects all passes that run after CPROP, including unrolling, IRA, the > scheduler, etc. > > There is already code that tries to avoid creating multi-entry loops: > > /* The irreducible loops created by redirecting of edges entering the > loop from outside would decrease effectiveness of some of the > following optimizations, so prevent this. */ > if (may_be_loop_header > && !(e->flags & EDGE_DFS_BACK)) > { > ei_next (&ei); > continue; > } > > Apparently your test case manages to slip through, and I wonder why. That's probably because even though BB 4 is a header, 3->4 and 9->4 are back edges (in the condition there's !(e->flags & EDGE_DFS_BACK), which in this case is 0). Note that the comment speaks about edges coming from outside of the loop. Updated patch: 2012-11-29 Marek Polacek PR middle-end/54838 * cprop.c (bypass_block): Set header and latch to NULL when BB has more than one latch edge. * gcc.dg/pr54838.c: New test. Marek --- gcc/cprop.c.mp 2012-11-29 15:49:53.120524295 +0100 +++ gcc/cprop.c 2012-11-29 17:45:03.004041242 +0100 @@ -1499,6 +1499,7 @@ bypass_block (basic_block bb, rtx setcc, int may_be_loop_header; unsigned removed_p; unsigned i; + unsigned n_back_edges; edge_iterator ei; insn = (setcc != NULL) ? setcc : jump; @@ -1510,13 +1511,12 @@ bypass_block (basic_block bb, rtx setcc, if (note) find_used_regs (&XEXP (note, 0), NULL); - may_be_loop_header = false; + n_back_edges = 0; FOR_EACH_EDGE (e, ei, bb->preds) if (e->flags & EDGE_DFS_BACK) - { - may_be_loop_header = true; - break; - } + n_back_edges++; + + may_be_loop_header = n_back_edges > 0; change = 0; for (ei = ei_start (bb->preds); (e = ei_safe_edge (ei)); ) @@ -1605,7 +1605,8 @@ bypass_block (basic_block bb, rtx setcc, && dest != EXIT_BLOCK_PTR) { if (current_loops != NULL - && e->src->loop_father->latch == e->src) + && (e->src->loop_father->latch == e->src + || n_back_edges > 1)) { /* ??? Now we are creating (or may create) a loop with multiple entries. Simply mark it for --- gcc/testsuite/gcc.dg/pr54838.c.mp 2012-11-26 14:48:43.783980854 +0100 +++ gcc/testsuite/gcc.dg/pr54838.c 2012-11-29 17:43:19.397737779 +0100 @@ -0,0 +1,24 @@ +/* PR middle-end/54838 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-forward-propagate -ftracer" } */ + +void bar (void); + +void +foo (void *b, int *c) +{ +again: + switch (*c) + { + case 1: + if (!b) + { + bar (); + return; + } + goto again; + case 3: + if (!b) + goto again; + } +}