From patchwork Thu Jan 19 11:22:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 717024 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 3v41dt45n0z9snk for ; Thu, 19 Jan 2017 22:22:26 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="vln8T1lI"; 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type:content-transfer-encoding; q=dns; s= default; b=y1RlVuoKC005AraIhA47ETJODSr1uLJYyLUAKM9IQDg8x5s824BrW 2qb1A9/SGsPmfF/pbCxYWaP6Pgf1UUtYux+waywUPOGy3h09w9Qfy6XNSUfw1Mso xRwFBpEgrqdrA8h3uJIdiCx+OvS4oPBoaed5b7vZgkjfVh082+FWrg= 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type:content-transfer-encoding; s= default; bh=COLDUf25iAL88vdYNOuCJqxUSX8=; b=vln8T1lInqSxh+SE1b5U OoRnCEpc70P/AoKouLR5dCplAbnq/Vgx4wnq3XVHCyDjcFaEXmNX1IgFb8o2jxal x/OK+PeFuZg+qx9JlUn8wj6EQOiT145NAmOFTPc3HQzEfm2W1h6+kKAI8D+D2GsO acsjRzSgTedPkwWH2SDUQDo= Received: (qmail 27969 invoked by alias); 19 Jan 2017 11:22:17 -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 27949 invoked by uid 89); 19 Jan 2017 11:22:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.0 required=5.0 tests=AWL, BAYES_50, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=bincheng, bin.cheng, Bin.Cheng, UD:Bin.Cheng X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-ot0-f194.google.com Received: from mail-ot0-f194.google.com (HELO mail-ot0-f194.google.com) (74.125.82.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 19 Jan 2017 11:22:06 +0000 Received: by mail-ot0-f194.google.com with SMTP id f9so3955642otd.0; Thu, 19 Jan 2017 03:22:06 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=bDv2Cq7orwXM1tvGQDij6XTQtzk82rse7RnG5kaGlco=; b=sv46G02ds25kWlzNrxMbj1ri/5+BdPCtfTojyNSfA7gaUKNGcoKaAJaOM39UaoYW3F m/Vy7L3yP6ZLXLMOyWnlQ73DJq90TzGzWNpq1rxnn+FQt+1QQ3y186DPZo+wGfxcfyGD 7D6k4C5alWcX3FWovf3TlnoJMamwvVwFb/c8PD6Z96PvRfjfchmSl3naDQgg9O7oKfbx CRkIZC7rx59+JX0jJ7MQRiBZ9qRqlDJbSLr9hQbQjFlX785YOLgxNTPFBTh61T6uhQQw 6DnUbuZzP1/fuljWiW7pWBZ3LiAhR3UuyvtTMzUUkZkLbHMGHoywXugdEBXA2mJlRdJx FysA== X-Gm-Message-State: AIkVDXLpyHU4hVHjY2OpL55kLptBVVRFwSlvf7GegWVkHvxsoVas+Dik9EBva5VYCsV2XPaSIspqEOwvwI6xLw== X-Received: by 10.157.11.37 with SMTP id a34mr3970218ota.258.1484824924891; Thu, 19 Jan 2017 03:22:04 -0800 (PST) MIME-Version: 1.0 Received: by 10.157.42.12 with HTTP; Thu, 19 Jan 2017 03:22:04 -0800 (PST) In-Reply-To: References: <2b986549-702b-2a09-84d5-e3f93518e4c3@suse.cz> From: Richard Biener Date: Thu, 19 Jan 2017 12:22:04 +0100 Message-ID: Subject: Re: [PATCH] Be careful about combined chain with length == 0 (PR, tree-optimization/70754). To: "Bin.Cheng" Cc: =?UTF-8?Q?Martin_Li=C5=A1ka?= , GCC Patches , vp@gcc.gnu.org X-IsSubscribed: yes On Thu, Jan 19, 2017 at 11:25 AM, Bin.Cheng wrote: > On Thu, Jan 19, 2017 at 9:42 AM, Richard Biener > wrote: >> On Wed, Jan 18, 2017 at 4:32 PM, Bin.Cheng wrote: >>> On Wed, Jan 18, 2017 at 2:54 PM, Richard Biener >>> wrote: >>>> On Wed, Jan 18, 2017 at 11:10 AM, Martin Liška wrote: >>>>> Hello. >>>>> >>>>> >>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>>> >>>>> Ready to be installed? >>>> >>>> I'm not sure. If we have such zero distance refs in the IL at the >>>> time pcom runs then not handling >>>> them will pessimize code-gen for cases where they are part of a larger >>>> chain. Esp. I don't like >>> Do you mean different chains distributed because of MAX_DISTANCE by >>> "larger chain"? With the patch, such chain of refs would still be >>> pred-commoned, just the arithmetic operation not combined, which could >>> be handled by later DOM? >>>> another stmt_dominates_stmt_p call and thus rather not handle length >>>> == 0 at all... >>> Not handle length == 0 chains at all may be sub-optimal. As you said, >>> such chain of refs at the point may simply because previous dom/cse >>> fail to analyze the references. >>>> >>>> We already seem to go great length in associating stuff when combining >>>> stuff thus isn't this >>>> maybe an artifact of this association? Maybe we simply need to sort >>>> the new chain after >>>> combining it so the root stmt comes last? >>>> >>>> Note that there seems to be only a single length per chain but not all >>>> refs in a chain need to >>>> have the same distance. This means your fix is likely incomplete? >>>> What prevents the situation >>>> to arise for distance != 0? >>> Yes, it's possible for two refs have the same distance in a chain with >>> length > 0. But that should not be a problem, because existing uses >>> are replaced by newly generated PHI variables which always dominate >>> the uses, right? >> >> I must admit I don't know predcom in such detail but then can we handle >> distance == 0 by simply inserting a PHI for those as well (a degenerate >> one of course)? Or can for distance == 0 the ref be not loop invariant? > Not sure if I understand the question correctly. Distance is > difference of niter between one ref and the root ref of the chain, so > 0 distance/length doesn't mean a loop invariant, it's very likely two > (exactly the same) references in each loop iteration, the address of > reference is still a SCEV. OTOH, invariant chain has invariant > address, and is handled separately. For the first question, it's > length, rather than distance that decides how the chain is handled. > For length > 0 chain, we have to insert PHIs to pass carried result of > memory reference, even some refs may have 0 distance to the root ref. >> >> Note that for length == 0 all refs in the chain will have a dependence distance >> of zero. So my first argument probably doesn't hold and we could simply >> remove handling of length == 0 chains and rely on CSE? > I am not sure, that CSE opportunity of references exists at this point > means previous cse pass failed for some reason. Or a later pass introduced it (in this case, the vectorizer). > Predcom could be the > only pass that can handle such case as it understands data reference > better. Note Martin's patch is not to skip handling of length == 0 > chain, later ref will still be CSEed with result of root ref, only the > combination operation like chain1 + chain2 is skipped. In this case, > following dom should be able to handle such (loop independent) cse > opportunities. I must admit I don't completely understand the consequences of this disabling but of course DOM should also be able to handle the CSE (ok, DOM is known to be quite weak with memory equivalence but it's not that data-dependence is really better in all cases). Can't we simply re-order refs in new_chain appropriately or handle this case in combinable_refs_p instead? That is, I understand the patch as a hack as it should be always possible to find dominating refs? In fact the point of the assert tells us a simpler fix may be which obviously matches the assert we run into for the testcase? I'd be ok with that (no stmt_dominates_stmt_p, heh) with a suitable comment before it. Richard. > > Thanks, > bin >> >> Richard. >> >>> Thanks, >>> bin >>>> >>>> Richard. >>>> >>>>> Martin >>>>> Index: tree-predcom.c =================================================================== --- tree-predcom.c (revision 244519) +++ tree-predcom.c (working copy) @@ -2330,6 +2334,12 @@ combine_chains (chain_p ch1, chain_p ch2 break; } } + if (new_chain->length == 0 + && ! new_chain->has_max_use_after) + { + release_chain (new_chain); + return NULL; + } ch1->combined = true; ch2->combined = true;