This is the mail archive of the 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: PATCH PING: stop ivopts from pessimizing code (PR42505)


>> 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


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