From patchwork Thu Jun 18 16:57:46 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 486437 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 5A1D81401DA for ; Fri, 19 Jun 2015 02:58:15 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=YM0YEux9; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=IO5UXguZvpbZhEGm7 WKN9YHbWXs2dGFR5GWhCxi324L0BzcVuAGsBEpqLoUSpGWTXpubSqY1rI++rQann aHEkCDJ9OhG3jSRlDIOFplK+AmncXnDXgNFcDvpHGFoJy725C9y565pEwQeMIg15 LDDrHIr+UiTi4pn1/4rld/Vmyo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=4WDPYYsJnyF3W7vMc8tM26D Lcqs=; b=YM0YEux9wrc9XScSv3cJEJ53P/aDHrfHhtuIiSl2ZSsGH2Nax7x1lmU quN3lUnWeRXZZ1PDYbqQc6knJepI/MrB1w62UUidcoPUjxpjUGFOPbfnb8oeT/dW 9pj6MAJ8caHyYBOHkcaLLbMLfxuOIUYwbjIetuWE2HOGT0zkug+s= Received: (qmail 67874 invoked by alias); 18 Jun 2015 16:58:06 -0000 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 Received: (qmail 67857 invoked by uid 89); 18 Jun 2015 16:58:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 18 Jun 2015 16:58:03 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1Z5d8Z-00054b-68 from Tom_deVries@mentor.com ; Thu, 18 Jun 2015 09:57:59 -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.3.224.2; Thu, 18 Jun 2015 17:57:57 +0100 Message-ID: <5582F88A.5030105@mentor.com> Date: Thu, 18 Jun 2015 18:57:46 +0200 From: Tom de Vries User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Richard Biener CC: GCC Patches , Jakub Jelinek , Richard Biener Subject: Re: [committed, gomp4] Rewrite virtuals into lcssa in transform_to_exit_first_loop_alt References: <55827255.60302@mentor.com> In-Reply-To: On 18/06/15 13:38, Richard Biener wrote: > On Thu, Jun 18, 2015 at 9:25 AM, Tom de Vries wrote: >> Hi, >> >> transform_to_exit_first_loop contains the following comment: >> ... >> /* Make sure that we have phi nodes on exit for all loop header phis >> (create_parallel_loop requires that). */ >> ... >> >> I ran into a problem where after transform_to_exit_first_loop_alt this >> property does not hold for virtuals, and we run into trouble in >> create_parallel_loop. >> >> The patch ensures that the property holds at the start of >> transform_to_exit_first_loop_alt, which has as effect that: >> - we simplify transform_to_exit_first_loop_alt a bit, and >> - since the property is kept by transform_to_exit_first_loop_alt, >> the property will be valid after transform_to_exit_first_loop_alt. >> >> Bootstrapped and reg-tested on x86_64. >> >> Committed to gomp-4_0-branch. >> >> OK for trunk? > > I once made virtual-operands loop-closed AFAICT, r190614, ... > but then somehow I didn't get it > correct (I suppose I ran into the trap of the existing machinery using > update_ssa to update out-of-loop uses) and removed that capability again. > ... and r195609. I guess not PR56150, but PR56113 is the related PR, see comment https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56113#c17 ? AFAICT from that PR, the problem was compilation speed, not a correctness problem? [ confusingly, the rewrite_into_loop_closed_ssa comment still claims to update virtuals: ... 1) Updating it during unrolling/peeling/versioning is trivial, since we do not need to care about the uses outside of the loop. The same applies to virtual operands which are also rewritten into loop closed SSA form. Note that virtual operands are always live until function exit. ... ] > So instead of adding another limited machinery inside tree-parloops > can you instead > make it available from tree-ssa-loop-manip.c, eventually sharing some code? > > Other passes would benefit from this as well and it could save some compile-time > as they wouldn't need to rewrite the virtual operands all the time. > Attached untested patch series contains these patches: 1. Move rewrite_virtuals_into_loop_closed_ssa to tree-ssa-loop-manip.c 2. Add bitmap_get_dominated_by 3. Add replace_uses_in_dominated_bbs 4. Add get_virtual_phi The last three patches split up the implementation of rewrite_virtuals_into_loop_closed_ssa in smaller, hopefully reusable bits. OK for gomp-4_0-branch and trunk if testing succeeds? I'm not sure what should be the next step. If you can point me to a use site in a pass that could benefit from using rewrite_virtuals_into_loop_closed_ssa, then I can try to take it from there. Thanks, - Tom [PATCH 4/4] Add get_virtual_phi 2015-06-18 Tom de Vries * tree-ssa-loop-manip.c (get_virtual_phi): Factor out of ... (rewrite_virtuals_into_loop_closed_ssa): ... here. --- gcc/tree-ssa-loop-manip.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c index 0d2c972..44c14df 100644 --- a/gcc/tree-ssa-loop-manip.c +++ b/gcc/tree-ssa-loop-manip.c @@ -603,44 +603,43 @@ replace_uses_in_dominated_bbs (tree old_val, tree new_val, basic_block bb) BITMAP_FREE (dominated); } -/* Ensure a virtual phi is present in the exit block, if LOOP contains a vdef. - In other words, ensure loop-closed ssa normal form for virtuals. */ +/* Return the virtual phi in BB. */ -void -rewrite_virtuals_into_loop_closed_ssa (struct loop *loop) +static gphi * +get_virtual_phi (basic_block bb) { gphi *phi; - edge exit = single_dom_exit (loop); phi = NULL; - for (gphi_iterator gsi = gsi_start_phis (loop->header); + for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi)) { if (virtual_operand_p (PHI_RESULT (gsi.phi ()))) { - phi = gsi.phi (); - break; + return gsi.phi (); } } + return NULL; +} + +/* Ensure a virtual phi is present in the exit block, if LOOP contains a vdef. + In other words, ensure loop-closed ssa normal form for virtuals. */ + +void +rewrite_virtuals_into_loop_closed_ssa (struct loop *loop) +{ + gphi *phi; + edge exit = single_dom_exit (loop); + + phi = get_virtual_phi (loop->header); if (phi == NULL) return; tree final_loop = PHI_ARG_DEF_FROM_EDGE (phi, single_succ_edge (loop->latch)); - phi = NULL; - for (gphi_iterator gsi = gsi_start_phis (exit->dest); - !gsi_end_p (gsi); - gsi_next (&gsi)) - { - if (virtual_operand_p (PHI_RESULT (gsi.phi ()))) - { - phi = gsi.phi (); - break; - } - } - + phi = get_virtual_phi (exit->dest); if (phi != NULL) { tree final_exit = PHI_ARG_DEF_FROM_EDGE (phi, exit); -- 1.9.1