[llvm] r184105 - Switch spill weights from a basic loop depth estimation to BlockFrequencyInfo.

[Jakob Stoklund Olesen]

On Jun 21, 2013, at 9:50 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:

> 
> On 21.06.2013, at 18:32, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
> 
>> 
>> On Jun 21, 2013, at 6:49 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>> 
>>> 
>>> On 18.06.2013, at 19:07, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
>>> 
>>>> The mustSpill() function identifies nodes that are doomed to spill no matter how many connected basic blocks prefer a register - it is predicting that update() will always compute Value = -1. It is used to prune the network so we don’t have to iterate over cold parts of the CFG. It’s a compile time optimization, but an important one.
>>>> 
>>>> It is also important that the function returns true for nodes with a MustSpill constraint.
>>>> 
>>>> The most accurate implementation is Bias < -sum(LinkWeights), the current version is faster by exploiting the knowledge that sum(LinkWeights) <= 2.0. With this patch, sum(LinkWeights) is a sum of block frequencies which can’t be assumed to be less than 2.0.
>>>> 
>>>> You could simply store the accumulated link weights for each node, that is probably the simplest solution.
>>> 
>>> You mean something like
>>>  return BiasN >= BlockFrequency::getMaxFrequency() ||
>>>         BiasN < BiasP + SumLinkWeights;
>>> 
>>> and add the frequency to SumLinkWeights every time addLink is called?
>> 
>> Yes, exactly.
>> 
>>> I tried this and it breaks tests.
>> 
>> I think it’s just a sign error. Try:
>> 
>> return BiasN >= BiasP + SumLinkWeights;
> 
> That doesn't help. The test that fails is CodeGen/ARM/lsr-unfolded-offset.ll (one of those that I had to fix when doing the initial BlockFrequency spill weight change), so it might be a harmless change in behavior.

Ideally, your first patch shouldn’t affect any behavior, but we are dealing with floating point, so small harmless changes could happen.

On the other hand, we may have broken something. I’ll take a closer look later today.

Thanks,
/jakob