[llvm-commits] BlockFrequency for BasicBlocks

[Andrew Trick]

On Jun 21, 2011, at 6:10 PM, Jakub Staszak wrote:

> The new version attached. OK to commit now?
> 
> On Jun 17, 2011, at 4:10 PM, Andrew Trick wrote:
> 
>> Hi Kuba,
>> 
>> If you define the constructor and destructor in the .cpp, then do you still need to include InitializePasses.h from BlockFrequency.h.
> Done, i also removed "LoopInfo.h"
> 
>> And do you really need a "deinit" method?
> Yes, if I try to delete BFI in destructor I get a warning ("note: neither the destructor nor the class-specific operator delete will be called, even if they are declared when the class is defined.").

I'm unclear why you have the ctor/dtor defined in the header. Try moving them to the .cpp.
> 
>> The block frequency pass should only require a *single* CFG graph traversal. I'm not sure how to do that with LLVM's current rpot/pot iterators. Ideally, LLVM would have a DFSOrder analysis to order the blocks. That ordering will remain valid until some CFG transform.
> I removed PO vector, so we have one RPO and one PO traversal now.
> 
>> For, now I think you can at least limit it to two traversals (one reverse postorder and one postorder) and add a FIXME so we remember to fold it into one traversal later. Subsequently whenever you need to visit blocks in RPO/PO order, just use the RPO/PO vectors directly.
>> 
>> Can you also comment that LoopSimplify can make the algorithm converge faster.
> Done.
> 
>> Can frequency reach zero in divBlockFreq? Don't you want to saturate at 1?
> I think it can, so i jest set it to 1/START_FREQ.

Yes, by "1" I meant literal 1, not scaled 1. I think this could happen inside getEdgeFreq() when (N * getBlockFreq(Src)) < D. You should probably check for that case and return the smallest nonzero frequency.

You actually shouldn't have a problem in divBlockFreq, but somewhere you should assert that the input BranchProbability is well-formed (N < D).

-Andy


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110621/4fe317aa/attachment.html>