From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 98348 invoked by alias); 28 May 2015 21:31:02 -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 98328 invoked by uid 89); 28 May 2015 21:31:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 28 May 2015 21:31:01 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 3F29B2C770C; Thu, 28 May 2015 21:30:59 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-36.phx2.redhat.com [10.3.113.36]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t4SLUw2Q012997; Thu, 28 May 2015 17:30:58 -0400 Message-ID: <55678912.1050000@redhat.com> Date: Thu, 28 May 2015 21:52:00 -0000 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Kugan , "gcc-patches@gcc.gnu.org" CC: Ilya Enkovich Subject: Re: [PR65768] Check rtx_cost when propagating constant References: <552E1907.4090708@linaro.org> <555436B3.6070900@linaro.org> In-Reply-To: <555436B3.6070900@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg02704.txt.bz2 I've CC'd Ilya as he's been looking at related issues in the x86 backend, but from the other direction and I think he ought to be aware of the interactions of this potential change and his work. In particular depending on the costing in the x86 backend we may see fewer propagations of GOTOFF constants to their use sites. On 05/13/2015 11:46 PM, Kugan wrote: > ping? > > Thanks, > Kugan > > On 15/04/15 17:53, Kugan wrote: >> As mentioned in PR65768, ARM gcc generates suboptimal code for constant >> Uses in loop. Part of the reason is cprop is undoing what loop invariant >> code motion did. >> >> Zhenqiang posted a patch at to fix this based on rtx costs: >> https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01321.html >> >> I cleaned it up and bootstrapped, regression tested on x86_64-linux-gnu; >> no new regressions. Is this OK for trunk? >> >> Thanks, >> Kugan >> >> gcc/ChangeLog: >> >> 2015-04-15 Kugan Vivekanandarajah >> Zhenqiang Chen >> >> PR target/65768 >> * cprop.c (try_replace_reg): Check cost of constants before propagating. So, I've reviewed the discussion from last year. To summarize my understanding (please correct me if I'm wrong): For various reasons we can have out-of-range constants for arithmetic, logical or other operations. Those out-of-range constants will typically be loaded into a register so that we can create valid insns. LICM (and code motion in general) may hoist the constant register loads out of loops, which we generally consider a win (there's certainly cases where it is not though). It's particularly helpful when the constant can be used by many instructions. Global constant propagation may then try to replace uses of the constant by the constant itself. Some of those propagations create valid insns, but insns with a higher cost than their prior form. This is effectively undoing LICM. The patch changes the constant propagator to check the rtx cost of the original form vs the propagated form and only propagates if the cost is the same or lower -- the obvious idea being to propagate the constant only when it saves us cycles. Please correct me if I've got the overall summary incorrect. There were several small issues raised that are probably worth a bit of further discussion. Register pressure. This patch can increase register pressure. It happens if, prior to this patch the constant was propagated to all the use sites. In that case the pseudo holding the constant is dead and gets eliminated. With this patch we may decline to propagate the constant to the use site (due to cost) and as a result the pseudo remains live, thus increasing register pressure. Based on Kugan's data, I don't see that as a major problem in practice. Though Ilya might have specific cases for i686 PIC where it's a bigger concern. Performance. There wasn't a big win with this patch on either tested architecture -- which is no great surprise. We're talking about very small cost differences, possibly differences that can be well hidden by modern pipelines. General conerns about using rtx costing. What Kugan is doing here is very similar to what's being done in other rtl passes WRT checking costs before making transformations. So I don't see that as a significant reason to object to the patch. WRT the patch itself. The "const_p" variable is poorly named, though I can kindof see how you settled on it. Maybe "check_rtx_costs" or something along those lines would be better. The comment for the second hunk would probably be better as: /* If TO is a constant, check the cost of the set after propagation to the cost of the set before the propagation. If the cost is higher, then do not replace FROM with TO. */ You should try to produce a testcase where this change shows a code generation improvement. Given we're checking target costs, that test will naturally be target specific. But please do try. So with the two nits fixed and a testcase, I think this can go forward. jeff