From patchwork Tue Oct 25 15:03:15 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Matz X-Patchwork-Id: 686505 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 3t3Gd138pRz9t2n for ; Wed, 26 Oct 2016 02:03:49 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=bAvmq9DF; 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:subject:message-id:mime-version:content-type; q=dns; s= default; b=b4nQGgeSlujeHm+e11UL2usnn4PLTNvAmaQWMLtteaOZlLqXky2bi DrroB/rmW9QBDyS3NxLz1J/2ElFe4CSCTPDdVkw4DWph8XdFL6Ksj9HzqmADfQyE r5XsqQwq0FcUh/MQMCTe7fc/EmAnNMEmY2F0mFdQHCnmFtCImO4fQg= 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:subject:message-id:mime-version:content-type; s= default; bh=oEz021fVPwlkuHKxFd/olNJuLfY=; b=bAvmq9DFLycZQsRam7h3 p1oWFgOdCj+91dXsJP+iOiklZnHxKDV35CkjBYwQra2c7+6P2gVTcIi0dP1ARM4W vK3zcKIkpu2L0zXpIcz5mNPQCOpG+CUUm5Ux5nv1RyJ1PF0GrHcEGfTTCrJsP7GL oSXmt5qschUJAqNp0W7nse0= Received: (qmail 27349 invoked by alias); 25 Oct 2016 15:03:31 -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 27020 invoked by uid 89); 25 Oct 2016 15:03:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=fulfill, 1n, 1N, IJ 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, 25 Oct 2016 15:03:20 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 110BCAABE for ; Tue, 25 Oct 2016 15:03:16 +0000 (UTC) Date: Tue, 25 Oct 2016 17:03:15 +0200 (CEST) From: Michael Matz To: gcc-patches@gcc.gnu.org Subject: Fix PR78060, PR78061, PR78088 Message-ID: User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 X-IsSubscribed: yes Hi, these are ICEs by the loop splitting pass. Two problems: using inconsistent types for operations and not dealing correctly with certain loop forms. First (78060 and 78088): I initially used an ideosyncratic way of calculating the new loop bound, needing several type switches that I haven't implemented correctly. Reordering the order of evaluations makes this easier and fixes the ICEs. Second (78061): I need to know the exit values of all loop phis. In simple loop forms (where the single exit is after the loop body) that's easy: it's the same as the value on the backedge. If the exit is in the middle of a loop that's not the case. This mattered only for virtual ops. All other values already needed a loop-closed phi-node before or in the latch block, which made it not empty which rejected the loop form. I've added a predicate that explicitely checks that the exit values are easily computable (i.e. that the back edge value is indeed also the exit value). Together regstrapped on x86-64, all languages + ada. Okay for trunk? Ciao, Michael. PR tree-optimization/78060 PR tree-optimization/78061 PR tree-optimization/78088 * tree-ssa-loop-split.c (easy_exit_values): New function. (tree_ssa_split_loops): Use it. (compute_new_first_bound): Change order of operations, fix invalid use of types. testsuite: * g++.dg/pr78060.C: New test. * gfortran.dg/pr78061.f: New test. * g++.dg/pr78088.C: New test. commit 10cbbd81f96d5183979dae647b77ee804179780e Author: Michael Matz Date: Mon Oct 24 17:14:08 2016 +0200 pr78060 pr78061 pr78088 diff --git a/gcc/testsuite/g++.dg/pr78060.C b/gcc/testsuite/g++.dg/pr78060.C new file mode 100644 index 0000000..d6cc7b3 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr78060.C @@ -0,0 +1,14 @@ +// PR tree-optimization/78060 +// { dg-do compile } +// { dg-options "-O3 -fsplit-loops" } +class A { +public: + template int &operator[](T2); +}; +int a; +A b; +void fn1() { + long c; + for (int l; l < c; ++l) + b[l] = l < 2 ?: a; +} diff --git a/gcc/testsuite/g++.dg/pr78088.C b/gcc/testsuite/g++.dg/pr78088.C new file mode 100644 index 0000000..1a5c063 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr78088.C @@ -0,0 +1,19 @@ +// PR tree-optimization/78088 +// { dg-do compile } +// { dg-options "-O3 -fsplit-loops" } +class A { +public: + int m_fn1(); +}; +struct B : A { + void m_fn2(); +}; +void B::m_fn2() { + long a; + int b, c; + for (;;) { + c = 0; + for (; c < a; ++c, ++b) + b > 0 ? m_fn1() : 0; + } +} diff --git a/gcc/testsuite/gfortran.dg/pr78061.f b/gcc/testsuite/gfortran.dg/pr78061.f new file mode 100644 index 0000000..7e4dd3d --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr78061.f @@ -0,0 +1,18 @@ +! { dg-do compile } +! { dg-options "-O3 -fsplit-loops" } + SUBROUTINE SSYMM(C) + REAL C(LDC,*) + LOGICAL LSAME + LOGICAL UPPER + IF (LSAME) THEN + DO 170 J = 1,N + DO 140 K = 1,J + IF (UPPER) THEN + END IF + 140 CONTINUE + DO 160 K = J + 1,N + C(I,J) = B(K) + 160 CONTINUE + 170 CONTINUE + END IF + END diff --git a/gcc/tree-ssa-loop-split.c b/gcc/tree-ssa-loop-split.c index 53abb36..dac68e6 100644 --- a/gcc/tree-ssa-loop-split.c +++ b/gcc/tree-ssa-loop-split.c @@ -190,13 +190,40 @@ find_or_create_guard_phi (struct loop *loop, tree guard_iv, affine_iv * /*iv*/) return NULL; } +/* Returns true if the exit values of all loop phi nodes can be + determined easily (i.e. that connect_loop_phis can determine them). */ + +static bool +easy_exit_values (struct loop *loop) +{ + edge exit = single_exit (loop); + edge latch = loop_latch_edge (loop); + gphi_iterator psi; + + /* Currently we regard the exit values as easy if they are the same + as the value over the backedge. Which is the case if the definition + of the backedge value dominates the exit edge. */ + for (psi = gsi_start_phis (loop->header); !gsi_end_p (psi); gsi_next (&psi)) + { + gphi *phi = psi.phi (); + tree next = PHI_ARG_DEF_FROM_EDGE (phi, latch); + basic_block bb; + if (TREE_CODE (next) == SSA_NAME + && (bb = gimple_bb (SSA_NAME_DEF_STMT (next))) + && !dominated_by_p (CDI_DOMINATORS, exit->src, bb)) + return false; + } + + return true; +} + /* This function updates the SSA form after connect_loops made a new edge NEW_E leading from LOOP1 exit to LOOP2 (via in intermediate conditional). I.e. the second loop can now be entered either via the original entry or via NEW_E, so the entry values of LOOP2 phi nodes are either the original ones or those at the exit of LOOP1. Insert new phi nodes in LOOP2 pre-header reflecting - this. */ + this. The loops need to fulfill easy_exit_values(). */ static void connect_loop_phis (struct loop *loop1, struct loop *loop2, edge new_e) @@ -383,37 +410,37 @@ compute_new_first_bound (gimple_seq *stmts, struct tree_niter_desc *niter, TREE_TYPE (controlbase), controlbase, controlstep); - /* Compute beg-guard_init. */ + /* Compute end-beg. */ + gimple_seq stmts2; + tree end = force_gimple_operand (niter->bound, &stmts2, + true, NULL_TREE); + gimple_seq_add_seq_without_update (stmts, stmts2); if (POINTER_TYPE_P (TREE_TYPE (enddiff))) { - tree tem = gimple_convert (stmts, sizetype, guard_init); + tree tem = gimple_convert (stmts, sizetype, enddiff); tem = gimple_build (stmts, NEGATE_EXPR, sizetype, tem); enddiff = gimple_build (stmts, POINTER_PLUS_EXPR, TREE_TYPE (enddiff), - enddiff, tem); + end, tem); } else enddiff = gimple_build (stmts, MINUS_EXPR, TREE_TYPE (enddiff), - enddiff, guard_init); + end, enddiff); - /* Compute end-(beg-guard_init). */ - gimple_seq stmts2; - tree newbound = force_gimple_operand (niter->bound, &stmts2, - true, NULL_TREE); - gimple_seq_add_seq_without_update (stmts, stmts2); - - if (POINTER_TYPE_P (TREE_TYPE (enddiff)) - || POINTER_TYPE_P (TREE_TYPE (newbound))) + /* Compute guard_init + (end-beg). */ + tree newbound; + enddiff = gimple_convert (stmts, TREE_TYPE (guard_init), enddiff); + if (POINTER_TYPE_P (TREE_TYPE (guard_init))) { enddiff = gimple_convert (stmts, sizetype, enddiff); enddiff = gimple_build (stmts, NEGATE_EXPR, sizetype, enddiff); newbound = gimple_build (stmts, POINTER_PLUS_EXPR, - TREE_TYPE (newbound), - newbound, enddiff); + TREE_TYPE (guard_init), + guard_init, enddiff); } else - newbound = gimple_build (stmts, MINUS_EXPR, TREE_TYPE (enddiff), - newbound, enddiff); + newbound = gimple_build (stmts, PLUS_EXPR, TREE_TYPE (guard_init), + guard_init, enddiff); /* Depending on the direction of the IVs the new bound for the first loop is the minimum or maximum of old bound and border. @@ -449,7 +476,6 @@ compute_new_first_bound (gimple_seq *stmts, struct tree_niter_desc *niter, build_int_cst (type2, addbound)); } - newbound = gimple_convert (stmts, TREE_TYPE (border), newbound); tree newend = gimple_build (stmts, minmax, TREE_TYPE (border), border, newbound); return newend; @@ -615,6 +641,7 @@ tree_ssa_split_loops (void) original exit before. */ && empty_block_p (loop->latch) && !optimize_loop_for_size_p (loop) + && easy_exit_values (loop) && number_of_iterations_exit (loop, single_exit (loop), &niter, false, true) && niter.cmp != ERROR_MARK