From patchwork Sun May 29 12:37:15 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 627500 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 3rHfRN2S7Cz9t67 for ; Sun, 29 May 2016 22:37:50 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=Nqmo0Okz; 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:from :subject:to:cc:message-id:date:mime-version:content-type; q=dns; s=default; b=OWNgPcXSpTrFAT2XErIbClqua85iz71q7CqN36GwAc2JRPd6PV emA0TwqahBaIZV9G+/yUBgxluByO/52OY6dOAjV6baCTxUtlu4w6PVMSo18y1eQy wUiziLVPpTxj7vbAqtTLksD388kz5AGKTwDD5Z0/IiiSLJfxOJxgJUEs0= 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:from :subject:to:cc:message-id:date:mime-version:content-type; s= default; bh=AGb9s5MmOI/Ix5LKeDPwL7+/XOc=; b=Nqmo0OkzWsT3RXLKM381 wRQg3rdouyQcIF5qHDp/lJNQf0vNxUfhZTXA/MIiXEaCl2KZP6+eUaTSLgwpoc3x 2b1XF5L+hRm7OxgqxeKc6NF0lHifugtkUMk0txEpUeijtoVAVieo4rkm+9/3UGnd kprnibiXZ1xyaPzu4ASh/BM= Received: (qmail 117074 invoked by alias); 29 May 2016 12:37:42 -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 117061 invoked by uid 89); 29 May 2016 12:37:41 -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=ce, gcc_assert, iii, III 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; Sun, 29 May 2016 12:37:30 +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 1b6zyA-0007E4-T0 from Tom_deVries@mentor.com ; Sun, 29 May 2016 05:37:27 -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; Sun, 29 May 2016 13:37:25 +0100 From: Tom de Vries Subject: [PATCH, PR69067] Remove assert in get_def_bb_for_const To: Sebastian Pop CC: GCC Patches , Richard Biener Message-ID: <574AE27B.7060205@mentor.com> Date: Sun, 29 May 2016 14:37:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 Hi, this patch fixes graphite PR69067, a 6/7 regression. I. Consider the following test-case, compiled with -O1 -floop-nest-optimize -flto: ... int a1, c1, cr, kt; int aa[2]; int ce (void) { while (a1 < 1) { int g8; for (g8 = 0; g8 < 3; ++g8) if (c1 != 0) cr = aa[a1 * 2] = kt; for (c1 = 0; c1 < 2; ++c1) aa[c1] = cr; ++a1; } return 0; } int main (void) { return ce (aa); } ... At graphite0, there's a loop with header bb4, which conditionally executes bb 5: ... : : # g8_39 = PHI <0(13), g8_19(3)> # cr_lsm.1_4 = PHI # cr_lsm.2_22 = PHI if (c1_lsm.3_35 != 0) goto ; else goto ; : aa[_3] = 0; : # cr_lsm.1_33 = PHI # cr_lsm.2_11 = PHI g8_19 = g8_39 + 1; if (g8_19 <= 2) goto ; else goto ; ... II. The graphite transformation moves the condition '(P_35 <= -1 || P_35 >= 1)' out of the loop: ... [scheduler] original ast: { for (int c0 = 0; c0 <= 2; c0 += 1) { S_4(c0); if (P_35 <= -1 || P_35 >= 1) S_5(c0); S_6(c0); } S_7(); for (int c0 = 0; c0 <= 1; c0 += 1) S_8(c0); } [scheduler] AST generated by isl: { if (P_35 <= -1) { for (int c0 = 0; c0 <= 2; c0 += 1) S_5(c0); } else if (P_35 >= 1) for (int c0 = 0; c0 <= 2; c0 += 1) S_5(c0); for (int c0 = 0; c0 <= 2; c0 += 1) { S_4(c0); S_6(c0); } S_7(); for (int c0 = 0; c0 <= 1; c0 += 1) S_8(c0); } ... When instantiating the ast back to gimple, we run into an assert: ... pr-graphite-4.c: In function ‘ce’: pr-graphite-4.c:5:1: internal compiler error: in get_def_bb_for_const, at graphite-isl-ast-to-gimple.c:1795 ce() ^ ... III. What happens is the following: in copy_cond_phi_args we try to copy the arguments of phi in bb6 to the arguments of new_phi in bb 46 ... (gdb) call debug_gimple_stmt (phi) cr_lsm.1_33 = PHI (gdb) call debug_gimple_stmt (new_phi) cr_lsm.1_62 = PHI <(28), (47)> ... While handling the '0' phi argument in add_phi_arg_for_new_expr we trigger this bit of code and call get_def_bb_for_const with bb.index == 46 and old_bb.index == 5: ... /* If the corresponding def_bb could not be found the entry will be NULL. */ if (TREE_CODE (old_phi_args[i]) == INTEGER_CST) def_pred[i] = get_def_bb_for_const (new_bb, gimple_phi_arg_edge (phi, i)->src); ... Neither of the two copies of bb 5 dominates bb 46, so we run into the assert at the end: ... /* Returns a basic block that could correspond to where a constant was defined in the original code. In the original code OLD_BB had the definition, we need to find which basic block out of the copies of old_bb, in the new region, should a definition correspond to if it has to reach BB. */ basic_block translate_isl_ast_to_gimple:: get_def_bb_for_const (basic_block bb, basic_block old_bb) const { vec *bbs = region->copied_bb_map->get (old_bb); if (!bbs || bbs->is_empty ()) return NULL; if (1 == bbs->length ()) return (*bbs)[0]; int i; basic_block b1 = NULL, b2; FOR_EACH_VEC_ELT (*bbs, i, b2) { if (b2 == bb) return bb; /* BB and B2 are in two unrelated if-clauses. */ if (!dominated_by_p (CDI_DOMINATORS, bb, b2)) continue; /* Compute the nearest dominator. */ if (!b1 || dominated_by_p (CDI_DOMINATORS, b2, b1)) b1 = b2; } gcc_assert (b1); return b1; } ... IV. Attached patch fixes this by removing the assert. Bootstrapped and reg-tested on x86_64. OK for trunk, 6-branch? Thanks, - Tom Remove assert in get_def_bb_for_const 2016-05-29 Tom de Vries PR tree-optimization/69067 * graphite-isl-ast-to-gimple.c (get_def_bb_for_const): Remove assert. * gcc.dg/graphite/pr69067.c: New test. --- gcc/graphite-isl-ast-to-gimple.c | 1 - gcc/testsuite/gcc.dg/graphite/pr69067.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/gcc/graphite-isl-ast-to-gimple.c b/gcc/graphite-isl-ast-to-gimple.c index 049a4c5..ff1d91f 100644 --- a/gcc/graphite-isl-ast-to-gimple.c +++ b/gcc/graphite-isl-ast-to-gimple.c @@ -1792,7 +1792,6 @@ get_def_bb_for_const (basic_block bb, basic_block old_bb) const b1 = b2; } - gcc_assert (b1); return b1; } diff --git a/gcc/testsuite/gcc.dg/graphite/pr69067.c b/gcc/testsuite/gcc.dg/graphite/pr69067.c new file mode 100644 index 0000000..d767381d --- /dev/null +++ b/gcc/testsuite/gcc.dg/graphite/pr69067.c @@ -0,0 +1,28 @@ +/* { dg-do link } */ +/* { dg-options " -O1 -floop-nest-optimize" } */ +/* { dg-additional-options "-flto" { target lto } } */ + +int a1, c1, cr, kt; +int aa[2]; + +int +ce (void) +{ + while (a1 < 1) + { + int g8; + for (g8 = 0; g8 < 3; ++g8) + if (c1 != 0) + cr = aa[a1 * 2] = kt; + for (c1 = 0; c1 < 2; ++c1) + aa[c1] = cr; + ++a1; + } + return 0; +} + +int +main (void) +{ + return ce (aa); +}