From patchwork Mon Jun 1 02:20:06 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 49275 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f70.google.com (mail-la0-f70.google.com [209.85.215.70]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id A9E5A218FC for ; Mon, 1 Jun 2015 02:20:43 +0000 (UTC) Received: by labc7 with SMTP id c7sf1209285lab.1 for ; Sun, 31 May 2015 19:20:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mailing-list:precedence:list-id :list-unsubscribe:list-archive:list-post:list-help:sender :delivered-to:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type:x-original-sender :x-original-authentication-results; bh=1cCy+q/LIwf6V4NA+hBLpNvqKRuTFBbCfPuM72Vl7t4=; b=a+0ENipKUs7xXtWLVnejCdzsKrR525uC0YrMGTH9dy/BilPOOjVatC25+UHnQ7jd/L uwvDnIHiIcO3NbN7iUf93B5laS92I00roxAO87wgu4z3xh+yJeKJVmn2fjJSCytgUeKB YUsHA3AUPwEoRvx7ni7JaCEXQgGCC1MvZqkVPJGlRM7XfPBeK5omaXSE6LCJVfPn3UYT DhQJNv6gyOclrI16i4QCANEkirQKIm6HeAVf94/DNmrhC52QUYxBQFOqa/osgYe+Ozlw sPYa4xb3FHaD7+2xjY4COcMnIRRwLhpww9DpxAgQ1hg/mGJhxFcxWXY3nTT9eXN4dnlZ bx4g== X-Gm-Message-State: ALoCoQkzFAAQJi1aS4go83KWLf4Zc3nHiCvbH58rcEptm4awLwza/QjreV+lTLlGaQiv0s4TtdYB X-Received: by 10.180.83.72 with SMTP id o8mr9384681wiy.3.1433125242347; Sun, 31 May 2015 19:20:42 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.7.202 with SMTP id l10ls592235laa.5.gmail; Sun, 31 May 2015 19:20:42 -0700 (PDT) X-Received: by 10.152.116.49 with SMTP id jt17mr18638381lab.82.1433125242054; Sun, 31 May 2015 19:20:42 -0700 (PDT) Received: from mail-la0-x22b.google.com (mail-la0-x22b.google.com. [2a00:1450:4010:c03::22b]) by mx.google.com with ESMTPS id f2si11092902laa.43.2015.05.31.19.20.41 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 31 May 2015 19:20:41 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c03::22b as permitted sender) client-ip=2a00:1450:4010:c03::22b; Received: by laat2 with SMTP id t2so90958458laa.1 for ; Sun, 31 May 2015 19:20:41 -0700 (PDT) X-Received: by 10.152.37.228 with SMTP id b4mr18467332lak.117.1433125241204; Sun, 31 May 2015 19:20:41 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.108.230 with SMTP id hn6csp1982614lbb; Sun, 31 May 2015 19:20:39 -0700 (PDT) X-Received: by 10.66.249.101 with SMTP id yt5mr36179261pac.116.1433125238818; Sun, 31 May 2015 19:20:38 -0700 (PDT) Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id py8si19217888pdb.157.2015.05.31.19.20.37 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 31 May 2015 19:20:38 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-399465-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Received: (qmail 40089 invoked by alias); 1 Jun 2015 02:20:25 -0000 Mailing-List: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Precedence: list 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 40070 invoked by uid 89); 1 Jun 2015 02:20:24 -0000 X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f51.google.com Received: from mail-pa0-f51.google.com (HELO mail-pa0-f51.google.com) (209.85.220.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 01 Jun 2015 02:20:22 +0000 Received: by padj3 with SMTP id j3so32706457pad.0 for ; Sun, 31 May 2015 19:20:21 -0700 (PDT) X-Received: by 10.68.170.229 with SMTP id ap5mr36503041pbc.132.1433125221078; Sun, 31 May 2015 19:20:21 -0700 (PDT) Received: from [10.1.1.3] (58-6-183-210.dyn.iinet.net.au. [58.6.183.210]) by mx.google.com with ESMTPSA id db1sm12491582pdb.50.2015.05.31.19.20.19 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 31 May 2015 19:20:20 -0700 (PDT) Message-ID: <556BC156.8040209@linaro.org> Date: Mon, 01 Jun 2015 12:20:06 +1000 From: Kugan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Jeff Law , "gcc-patches@gcc.gnu.org" Subject: Re: [PR65768] Check rtx_cost when propagating constant References: <552E1907.4090708@linaro.org> <555436B3.6070900@linaro.org> <5567892C.2010907@redhat.com> <5568081A.4090103@linaro.org> <5569429C.3050200@redhat.com> In-Reply-To: <5569429C.3050200@redhat.com> X-IsSubscribed: yes X-Original-Sender: kugan.vivekanandarajah@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c03::22b as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=pass header.i=@gcc.gnu.org X-Google-Group-Id: 836684582541 On 30/05/15 14:54, Jeff Law wrote: > On 05/29/2015 12:32 AM, Kugan wrote: >>>>> >>>>> PR target/65768 >>>>> * cprop.c (try_replace_reg): Check cost of constants before >>>>> propagating. >>> I should have also noted, fresh bootstrap & regression test is needed >>> too. >> >> Thanks Jeff for the comments. I did a fresh bootstrap and regression >> testing on x86_64-linux-gnu with no new regression. I will wait for >> you ACK. > Can you address the 3 issues in my prior message? I'll include them > here for clarity: > > -- > > 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. > -- > Thanks Jeff and apologies for missing your previous email. I have now fixed the comments as you suggested and changed the PR target/65768 testcase such that it tests this case. I will commit it if there is no objections to this. Thanks, Kugan gcc/ChangeLog: 2015-06-01 Kugan Vivekanandarajah Zhenqiang Chen PR target/65768 * cprop.c (try_replace_reg): Check cost of constants before propagating. gcc/testsuite/ChangeLog: 2015-06-01 Kugan Vivekanandarajah PR target/65768 * gcc.target/arm/maskdata.c: Remove -fno-gcse. diff --git a/gcc/cprop.c b/gcc/cprop.c index 57c44ef..94bb064 100644 --- a/gcc/cprop.c +++ b/gcc/cprop.c @@ -765,12 +765,37 @@ try_replace_reg (rtx from, rtx to, rtx_insn *insn) int success = 0; rtx set = single_set (insn); + bool check_rtx_costs = true; + bool speed = optimize_bb_for_speed_p (BLOCK_FOR_INSN (insn)); + int old_cost = set ? set_rtx_cost (set, speed) : 0; + + if ((note != 0 + && REG_NOTE_KIND (note) == REG_EQUAL + && (GET_CODE (XEXP (note, 0)) == CONST + || CONSTANT_P (XEXP (note, 0)))) + || (set && CONSTANT_P (SET_SRC (set)))) + check_rtx_costs = false; + /* Usually we substitute easy stuff, so we won't copy everything. We however need to take care to not duplicate non-trivial CONST expressions. */ to = copy_rtx (to); validate_replace_src_group (from, to, insn); + + /* 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. */ + + if (check_rtx_costs + && CONSTANT_P (to) + && (set_rtx_cost (set, speed) > old_cost)) + { + cancel_changes (0); + return false; + } + + if (num_changes_pending () && apply_change_group ()) success = 1; diff --git a/gcc/testsuite/gcc.target/arm/maskdata.c b/gcc/testsuite/gcc.target/arm/maskdata.c index 6d6bb39..35d2f06 100644 --- a/gcc/testsuite/gcc.target/arm/maskdata.c +++ b/gcc/testsuite/gcc.target/arm/maskdata.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options " -O2 -fno-gcse " } */ +/* { dg-options " -O2" } */ /* { dg-require-effective-target arm_thumb2_ok } */ #define MASK 0xff00ff