From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28328 invoked by alias); 30 Nov 2012 09:01:46 -0000 Received: (qmail 28318 invoked by uid 22791); 30 Nov 2012 09:01:45 -0000 X-SWARE-Spam-Status: No, hits=-5.0 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-oa0-f47.google.com (HELO mail-oa0-f47.google.com) (209.85.219.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 30 Nov 2012 09:01:38 +0000 Received: by mail-oa0-f47.google.com with SMTP id h1so198586oag.20 for ; Fri, 30 Nov 2012 01:01:38 -0800 (PST) MIME-Version: 1.0 Received: by 10.182.235.46 with SMTP id uj14mr441006obc.40.1354266097892; Fri, 30 Nov 2012 01:01:37 -0800 (PST) Received: by 10.76.95.202 with HTTP; Fri, 30 Nov 2012 01:01:37 -0800 (PST) In-Reply-To: <5890792.ZMS3qalP0H@polaris> References: <20121126142843.GH17362@redhat.com> <20121129153852.GC10621@redhat.com> <5890792.ZMS3qalP0H@polaris> Date: Fri, 30 Nov 2012 09:02:00 -0000 Message-ID: Subject: Re: [PATCH] Don't bypass blocks with multiple latch edges (PR middle-end/54838) From: Richard Biener To: Eric Botcazou Cc: Marek Polacek , gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-11/txt/msg02518.txt.bz2 On Thu, Nov 29, 2012 at 6:43 PM, Eric Botcazou wrote: >> Yikes, sorry, it wasn't clear to me what PROP_loops really does. Anyway, >> I think I have a better fix now. The problem is just that when removing >> BB 4 (which was a header), we have to zap ->header and ->latch. We >> already have code for this: >> >> if (current_loops != NULL >> && e->src->loop_father->latch == e->src) >> { >> /* ??? Now we are creating (or may create) a loop >> with multiple entries. Simply mark it for >> removal. Alternatively we could not do this >> threading. */ >> e->src->loop_father->header = NULL; >> e->src->loop_father->latch = NULL; >> } >> >> but the thing is that when there are multiple latch edges, then >> ->latch is NULL. So we need to keep track of how many latch edges >> the header has. Regtested/bootstrapped on x86_64, ok for trunk? >> >> Can I get rid of may_be_loop_header (and just use n_latch_edges > 0 >> instead at that one place) in a followup? >> >> 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. > > This looks good on principle, but can't we do better now that we have the loop > structure? Can't we compute is_loop_header instead of may_be_loop_header and > simplify the condition under which we mark the loop for removal? Or maybe we > should call disambiguate_loops_with_multiple_latches on entry of the pass? > > Richard, what would be the "modern" approach to solving the problem here? RTL cprop seems to run both before and after RTL loop optimizers (currently after RTL loop optimizers we throw away loops - an arbitrary chosen point before IRA across which I could not get things to work). Thus you could do if (current_loops) is_loop_header = bb == bb->loop_father->header; else { may_be_loop_header = false; FOR_EACH_EDGE (e, ei, bb->preds) if (e->flags & EDGE_DFS_BACK) { may_be_loop_header = true; break; } } I don't understand /* 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; } why isn't this simply if (may_be_loop_header) { ei_next (&ei); continue; } ? It looks like the code tries to allow "rotating" a loop - but that's only good if bb has exactly two predecessors (one entry and one latch edge). And even then it requires to manually update the loop structures (update what the new header and latch blocks are). That said, removing the !(e->flags & EDGE_DFS_BACK) condition seems to fix the ICE. Threading across a loop header is in fact complicated (see the special routine tree-ssa-threadupdate.c:thread_through_loop_header necessary for that). Let's declare the GIMPLE level did all interesting threadings through headers. Btw, if a pass wants to look at anything loop related _besides_ loop headers it has to call loop_optimizer_init. The only thing that is guaranteed to be kept up-to-date when PROP_loops is set (and thus current_loops != NULL) is the header field in the loop tree. Richard. > -- > Eric Botcazou