From patchwork Tue Jan 31 15:11:41 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 722005 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 3vCV9t41nQz9sCM for ; Wed, 1 Feb 2017 02:12:33 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="O2uPvkIQ"; 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:date :from:to:cc:subject:in-reply-to:message-id:references :mime-version:content-type; q=dns; s=default; b=xw4ZkcvcjgaPCagf gwVHUmxlaA5WGYKYzgU7oNvShREI3/k1noEOBI4MkdGX4nm0l4CQJ83DlFPIu6fS /X5/JHsJkinpNFxuSL5cNP61D70xGy5/L6k8osMHdOa1oSoRClBIV0nyUUAOmFIt tBWCJKfWPrPiGlq0UDJpyGCpevI= 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:date :from:to:cc:subject:in-reply-to:message-id:references :mime-version:content-type; s=default; bh=YxhKP7+FJbLMpNxxPA1Wi+ cP2co=; b=O2uPvkIQPUBXp/Xo44FGjG/YHod4F7MIvkjVW64gFpgzXO8QqxBWxS llypU4Y0BLlHjybMxDnKFxNAoa2Q6ztH9rs0UXnkVviYXCL/eK2fwu46e0ODgZUF 5KNKF0ICXIwZMfa+IBwZ78HEMs16JRaPu4WkjmEfPmjSzLtV7zg84= Received: (qmail 85013 invoked by alias); 31 Jan 2017 15:12:02 -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 79894 invoked by uid 89); 31 Jan 2017 15:11:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 31 Jan 2017 15:11:44 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 4E56DAC46; Tue, 31 Jan 2017 15:11:41 +0000 (UTC) Date: Tue, 31 Jan 2017 16:11:41 +0100 (CET) From: Richard Biener To: Sebastian Pop cc: GCC Patches Subject: Re: [PATCH] Fix PR71824 In-Reply-To: Message-ID: References: User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 On Tue, 31 Jan 2017, Richard Biener wrote: > On Tue, 31 Jan 2017, Sebastian Pop wrote: > > > Resend as plain text to please gcc-patches@ > > > > On Tue, Jan 31, 2017 at 8:39 AM, Sebastian Pop wrote: > > > > > > > > > On Tue, Jan 31, 2017 at 7:49 AM, Richard Biener wrote: > > >> > > >> > > >> The following fixes an ICE that happens because instantiate_scev > > >> doesn't really work as expected for SESE regions (a FIXME comment > > >> hints at this already). So instead of asserting all goes well > > >> just bail out (add_loop_constraints seems to add constraints not > > >> required for correctness?). > > > > > > > > > The conditions under which a loop is executed are required for correctness. > > > There is a similar check in scop_detection::can_represent_loop_1 > > > > > > && (niter = number_of_latch_executions (loop)) > > > && !chrec_contains_undetermined (niter) > > > > > > that is supposed to filter out all these loops where this assert does not > > > hold. > > > The question is: why scop detection has not rejected this loop? > > > > > > Well, I see that we do not check that niter can be analyzed in the region: > > > so we would need another check like this: > > > > > > diff --git a/gcc/graphite-scop-detection.c b/gcc/graphite-scop-detection.c > > > index 3860693..8e14412 100644 > > > --- a/gcc/graphite-scop-detection.c > > > +++ b/gcc/graphite-scop-detection.c > > > @@ -931,6 +931,7 @@ scop_detection::can_represent_loop_1 (loop_p loop, > > > sese_l scop) > > > && niter_desc.control.no_overflow > > > && (niter = number_of_latch_executions (loop)) > > > && !chrec_contains_undetermined (niter) > > > + && !chrec_contains_undetermined (scalar_evolution_in_region (scop, > > > loop, niter)) > > > && graphite_can_represent_expr (scop, loop, niter); > > > } > > > > > > Could you please try this patch and see whether it fixes the problem? > > It doesn't. It seems we test the above before the regions are > eventually merged? That is, the above enters with > > $46 = (const sese_l &) @0x7fffffffd6f0: { > entry = 7)>, > exit = 8)>} > > but the failing case with > > $15 = (const sese_l &) @0x298b420: {entry = > 3)>, > exit = 15)>} seems to fix it. Richard. Index: graphite-scop-detection.c =================================================================== --- graphite-scop-detection.c (revision 245064) +++ graphite-scop-detection.c (working copy) @@ -905,7 +905,9 @@ scop_detection::build_scop_breadth (sese sese_l combined = merge_sese (s1, s2); - if (combined) + if (combined + && loop_is_valid_in_scop (loop, combined) + && loop_is_valid_in_scop (loop->next, combined)) s1 = combined; else add_scop (s2); @@ -931,6 +933,8 @@ scop_detection::can_represent_loop_1 (lo && niter_desc.control.no_overflow && (niter = number_of_latch_executions (loop)) && !chrec_contains_undetermined (niter) + && !chrec_contains_undetermined (scalar_evolution_in_region (scop, + loop, niter)) && graphite_can_represent_expr (scop, loop, niter); }