From patchwork Wed Mar 16 10:16:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 598225 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 3qQ6q73FPyz9sCj for ; Wed, 16 Mar 2016 21:17:04 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=NknHPrCz; 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 :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type:content-transfer-encoding; q=dns; s= default; b=tRxMupgbepbW6gu7S7iQq/zcVCymNzh3GCNinVaex3HRUOIWqNGCh 0aSnK7DMs9e+Ql/B0nZWQiQWPK2Dyh//y7Qfu7Mba7aIgW2vW0oNX5GPnTXUgdIt OvIXdii10CYEbuPLCvVfxjB5wRNAOnQ8+qr3fraCNS5tGeJ91I9eN0= 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 :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type:content-transfer-encoding; s=default; bh=gS5GA93A3mqY/Uf6+n3M3dWIKH0=; b=NknHPrCzVLM7SuiUEEnUKN+20fPP KShc4COmOljp0AE0ewhah+hpQfVIrYVIFwizH4aAN7X2d3VdKt/NiQ6GdpQ6+6+5 W4YTUixPMF/Rws6KiETCiKmKKkpONH57wsmRIwaZG4sAAk6Ol67y3ubaIIcU2RaK SoX4woojrX31e7A= Received: (qmail 123384 invoked by alias); 16 Mar 2016 10:16:55 -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 123366 invoked by uid 89); 16 Mar 2016 10:16:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=redirect, incoming 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 16 Mar 2016 10:16:52 +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 1ag8VV-0004ve-EO from Tom_deVries@mentor.com ; Wed, 16 Mar 2016 03:16:49 -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; Wed, 16 Mar 2016 10:16:48 +0000 Subject: Re: [PATCH, PR68715] Add missing single_pred_p test in scop_detection::merge_sese To: Richard Biener References: <56E9120A.7040404@mentor.com> CC: Sebastian Pop , GCC Patches From: Tom de Vries Message-ID: <56E9327B.9040108@mentor.com> Date: Wed, 16 Mar 2016 11:16:27 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: On 16/03/16 09:53, Richard Biener wrote: > Hmm, it looks like for all what this function does this effectively > pessimizes scop merging and it would be easier to split 'exit' > in case its destination is unsuitable (not trivially empty). > Agreed. > The > > /* For now we just want to bail out when exit does not post-dominate > entry. > TODO: We might just add a basic_block at the exit to make exit > post-dominate entry (the entire region). */ > if (!dominated_by_p (CDI_POST_DOMINATORS, get_entry_bb (combined), > get_exit_bb (combined)) > || !dominated_by_p (CDI_DOMINATORS, get_exit_bb (combined), > > comment also suggests that splitting the get_nearest_pdom_with_single_exit > edge and including the new BB in the combined region would also always fix > the dominance relation (though I don't see how it could do that and > the comment looks wrong and by construction the check should never > trigger). > I've replaced the entire condition with two asserts: ... /* FIXME: We should remove this piece of code once canonicalize_loop_closed_ssa has been removed, because that function ... and ran graphite.exp. I ran into an ICE for pr68693.f90, for the second assert. The exit bb has two incoming edges from within the sese, and one from outside. Given that the exit bb is empty, we could split the exit edge and redirect the edge from outside the sese to the new bb, which would fix the sese. Perhaps that is what is meant in the comment, I'm not sure. Thanks, - Tom diff --git {a,b}/gcc/graphite-scop-detection.c index 7615842..762a248 100644 --- a/gcc/graphite-scop-detection.c +++ b/gcc/graphite-scop-detection.c @@ -820,14 +820,10 @@ scop_detection::merge_sese /* For now we just want to bail out when exit does not post-dominate entry. TODO: We might just add a basic_block at the exit to make exit post-dominate entry (the entire region). */ - if (!dominated_by_p (CDI_POST_DOMINATORS, get_entry_bb (combined), - get_exit_bb (combined)) - || !dominated_by_p (CDI_DOMINATORS, get_exit_bb (combined), - get_entry_bb (combined))) - { - DEBUG_PRINT (dp << "[scop-detection-fail] cannot merge " - "seses.\n"); - return invalid_sese; - } + gcc_assert (dominated_by_p (CDI_POST_DOMINATORS, + get_entry_bb (combined), + get_exit_bb (combined))); + gcc_assert (dominated_by_p (CDI_DOMINATORS, + get_exit_bb (combined), + get_entry_bb (combined)));