[LLVMdev] BlockFrequency spill weights

[Benjamin Kramer]

[Splitting this out from the original thread to reduce noise in it]


On 17.06.2013, at 18:43, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:

> 
> On Jun 17, 2013, at 7:03 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
> 
>> 
>> On 17.06.2013, at 15:56, Diego Novillo <dnovillo at google.com> wrote:
>> 
>>> On 2013-06-15 16:39 , Benjamin Kramer wrote:
>>>> Do you want to take over this effort or should I poke more at it?
>>> 
>>> Since you've already started, it's easier if you poke more at it. Thanks.  I've got a whole bunch of other things to go through.
>> 
>> OK, will do.
>> 
>> 
>> Jakob any comments on the patch? The only interesting part is in LiveIntervals::getSpillWeight. Do we have to scale the values somehow there?
> 
> Yes, BlockFrequency::getFrequency() is poorly named, it returns a fixpoint number. I think you should scale it to be relative to the entry block frequency.
> 
> +LiveIntervals::getSpillWeight(bool isDef, bool isUse, BlockFrequency freq) {
> +  return (isDef + isUse) * freq.getFrequency();
> }
> 
> This computation can overflow.

Yep, I went down the easy route and converted it to floating point arithmetic. Is that OK here?

> 
> @@ -178,9 +180,10 @@ bool SpillPlacement::runOnMachineFunction(MachineFunction &mf) {
> 
>   // Compute total ingoing and outgoing block frequencies for all bundles.
>   BlockFrequency.resize(mf.getNumBlockIDs());
> +  MachineBlockFrequencyInfo &MBFI = getAnalysis<MachineBlockFrequencyInfo>();
>   for (MachineFunction::iterator I = mf.begin(), E = mf.end(); I != E; ++I) {
> -    float Freq = LiveIntervals::getSpillWeight(true, false,
> -                                               loops->getLoopDepth(I));
> +    float Freq =
> +        LiveIntervals::getSpillWeight(true, false, MBFI.getBlockFreq(I));
>     unsigned Num = I->getNumber();
>     BlockFrequency[Num] = Freq;
> 
> I think you should leave LiveIntervals out of this and just the block frequency directly, scaled as above. The getSpillWeight() function is just used here to get the non-overflowing frequency approximation.

Fixed.

> 
> Otherwise, looks good.

The attached patch also fixes the regression tests that failed. Mostly minor changes in register allocation. The SPARC one is a bit scary because an instruction now gets moved across inline assembly into a delay slot. Do you know if that is a safe transformation?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: block-frequency-spilling.patch
Type: application/octet-stream
Size: 25635 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20130617/351386be/attachment.obj>
-------------- next part --------------


- Ben