This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: [PR65768] Check rtx_cost when propagating constant

From: Jeff Law <law at redhat dot com>
To: Kugan <kugan dot vivekanandarajah at linaro dot org>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
Date: Tue, 02 Jun 2015 12:55:19 -0600
Subject: Re: [PR65768] Check rtx_cost when propagating constant
Authentication-results: sourceware.org; auth=none
References: <552E1907 dot 4090708 at linaro dot org> <555436B3 dot 6070900 at linaro dot org> <5567892C dot 2010907 at redhat dot com> <5568081A dot 4090103 at linaro dot org> <5569429C dot 3050200 at redhat dot com> <556BC156 dot 8040209 at linaro dot org>

On 05/31/2015 08:20 PM, Kugan wrote:



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.

No objections.  Thanks for your patience on this!

jeff

References:
- Re: [PR65768] Check rtx_cost when propagating constant
  - From: Kugan

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]