Re: [PR65768] Check rtx_cost when propagating constant

[Jeff Law <law@redhat.com>]

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