[LLVMdev] RFC - Profile Guided Optimization in LLVM

[Jakob Stoklund Olesen]

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.

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

Otherwise, looks good.

Thanks,
/jakob