PATCH PING: stop ivopts from pessimizing code (PR42505)

H.J. Lu hjl.tools@gmail.com
Tue Jul 6 15:25:00 GMT 2010


On Mon, Jul 5, 2010 at 9:43 AM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
> Zdenek Dvorak wrote:
>
>>>> As for this change:
>>>>
>>>>>>    * cfgloopanal.c (init_set_costs): Use call_used_regs rather than
>>>>>>    fixed_regs to count number of registers available for loop
>>>>>> variables.
>>>>
>>>> Should not we make call_used_regs unavailable only if there is a
>>>> function call in the
>>>> loop?
>>>
>>> I actually tried that before submitting the patch, and saw that
>>> empirically the results were better using call_used_regs all the time.  My
>>> theory is that, at least on ARM, a couple additional scratch registers are
>>> required for loading constants and performing some of the address
>>> computations that ivopts thinks are free.  It is better to be conservative
>>> in ivopts and underestimate register availability rather than risk
>>> introducing additional spills, which would totally overwhelm any savings
>>> from ivopts.
>>>
>>> I realize that it's possible to tweak the costs model in other ways, but
>>> after a week or two poking at it I felt like it was a black hole.  The
>>> minimal change I proposed fixes a fairly obvious bug in the current code in
>>> a conservatively correct way, and actually helped both code size and speed,
>>> unlike other things I tried.  ;-)  Perhaps someone else would like to take a
>>> stab at it and/or can demonstrate that the more complicated version that
>>> tests for a function call in the loop wins on some other target, even if it
>>> produces worse code on ARM than my simple fix?
>>
>> my concern is that this change is not obviously correct, and did not get
>> enough testing to warrant it purely on its heuristic value.  So, I cannot
>> approve it unless someone picks it up and makes a more detailed analysis
>> of the
>> problem,
>
> Hrmmmm.  Well, the current code is obviously incorrect, so I don't think
> it's right to do nothing, either.  I guess I'll check in the other
> uncontroversial part of the patch to tree-ssa-loop-ivopts.c first to get
> that out of the way, and then try resurrecting the more complicated hack for
> init_set_costs.
>

Your checkin:

http://gcc.gnu.org/ml/gcc-cvs/2010-07/msg00198.html

caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44838


-- 
H.J.



More information about the Gcc-patches mailing list