From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69075 invoked by alias); 20 Apr 2015 12:26:01 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 69065 invoked by uid 89); 20 Apr 2015 12:26:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Mon, 20 Apr 2015 12:26:00 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id CBBF8AC92; Mon, 20 Apr 2015 12:25:56 +0000 (UTC) Date: Mon, 20 Apr 2015 12:26:00 -0000 From: Richard Biener To: Tom de Vries cc: GCC Patches , Jakub Jelinek Subject: Re: [PING][PATCH][PR65443] Add transform_to_exit_first_loop_alt In-Reply-To: <552E69ED.7020601@mentor.com> Message-ID: References: <551564D0.2090308@mentor.com> <551E8A1F.5050908@mentor.com> <552E69ED.7020601@mentor.com> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2015-04/txt/msg01016.txt.bz2 On Wed, 15 Apr 2015, Tom de Vries wrote: > On 03-04-15 14:39, Tom de Vries wrote: > > On 27-03-15 15:10, Tom de Vries wrote: > > > Hi, > > > > > > this patch fixes PR65443, a todo in the parloops pass for function > > > transform_to_exit_first_loop: > > > ... > > > TODO: the common case is that latch of the loop is empty and > > > immediately > > > follows the loop exit. In this case, it would be better not to copy > > > the > > > body of the loop, but only move the entry of the loop directly before > > > the > > > exit check and increase the number of iterations of the loop by one. > > > This may need some additional preconditioning in case NIT = ~0. > > > ... > > > > > > The current implementation of transform_to_exit_first_loop transforms the > > > loop > > > by moving and duplicating the loop body. This patch transforms the loop > > > into the > > > same normal form, but without the duplication, if that's possible > > > (determined by > > > try_transform_to_exit_first_loop_alt). > > > > > > The actual transformation, done by transform_to_exit_first_loop_alt > > > transforms > > > this loop: > > > ... > > > bb preheader: > > > ... > > > goto > > > > > > : > > > ... > > > if (ivtmp < n) > > > goto ; > > > else > > > goto ; > > > > > > : > > > ivtmp = ivtmp + inc; > > > goto > > > ... > > > > > > into this one: > > > ... > > > bb preheader: > > > ... > > > goto > > > > > > : > > > ... > > > goto ; > > > > > > : > > > if (ivtmp < n + 1) > > > goto ; > > > else > > > goto ; > > > > > > : > > > ivtmp = ivtmp + inc; > > > goto > > > ... > > > > > > > Updated patch, now using redirect_edge_var_map and flush_pending_stmts. > > > > Bootstrapped and reg-tested on x86_64. > > > > OK for stage1 trunk? > > > > Ping. +static void +replace_imm_uses (tree val, imm_use_iterator *imm_iter) +{ + use_operand_p use_p; + + FOR_EACH_IMM_USE_ON_STMT (use_p, *imm_iter) + SET_USE (use_p, val); Use propagate_value. Why this odd interface passing a imm_iter?! In fact most of the "repair SSA" in transform_to_exit_first_loop_alt looks too ad-hoc to me ... it also looks somewhat familiar to other code ... Unfortunately the comment before the function isn't in SSA form so it's hard to follow the transform. I consider the parloops code bitrotten, no repair possible, so I might be convinced to not care about new spaghetti in there... + /* Fix up loop structure. TODO: Check whether this is sufficient. */ + loop->header = new_header; + no, surely not. Number of iterations (and estimates) are off after the transform and loop->latch might also need updating. "Safest" is to simply schedule a fixup (but you'll lose any loop annotation in that process). + /* Figure out whether nit + 1 overflows. */ + if (TREE_CODE (nit) == INTEGER_CST) + { + if (!tree_int_cst_equal (nit, TYPE_MAXVAL (nit_type))) in case nit_type is a pointer type TYPE_MAXVAL will be NULL I think. Is the whole exercise for performance? In that case using an entirely new, unsigned IV, that runs from 0 to niter should be easiest and just run-time guard that niter == +INF case? For the graphite case, can't you make graphite generate the loops exit-first in the first place? The testcases are just correctness ones? That is, there isn't any testcase that checks whether the new code is exercised? (no extra debugging dumping?) Thanks, Richard.