
<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Jun 21, 2011, at 6:10 PM, Jakub Staszak wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">The new version attached. OK to commit now?<div><br><div><div>On Jun 17, 2011, at 4:10 PM, Andrew Trick wrote:</div><br><blockquote type="cite"><div>Hi Kuba,<br><br>If you define the constructor and destructor in the .cpp, then do you still need to include InitializePasses.h from BlockFrequency.h. </div></blockquote><div>Done, i also removed "LoopInfo.h"</div><br><blockquote type="cite"><div>And do you really need a "deinit" method?<br></div></blockquote><div>Yes, if I try to delete BFI in destructor I get a warning ("<span class="Apple-style-span" style="font-size: 12px; ">note: neither the destructor nor the class-specific operator delete will be called, even if they are declared when the class is defined.").</span></div></div></div></div></blockquote><div><br></div><div>I'm unclear why you have the ctor/dtor defined in the header. Try moving them to the .cpp.</div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div><br><blockquote type="cite"><div>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.<font class="Apple-style-span"><font class="Apple-style-span" color="#144FAE"><br></font></font></div></blockquote>I removed PO vector, so we have one RPO and one PO traversal now.<br><br><blockquote type="cite"><div>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.<br><br>Can you also comment that LoopSimplify can make the algorithm converge faster.<br></div></blockquote>Done.</div><div><br><blockquote type="cite"><div>Can frequency reach zero in divBlockFreq? Don't you want to saturate at 1?<br></div></blockquote><div>I think it can, so i jest set it to 1/START_FREQ.</div></div></div></div></blockquote><br></div><div><div>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.</div><div><br></div><div>You actually shouldn't have a problem in divBlockFreq, but somewhere you should assert that the input BranchProbability is well-formed (N < D).</div><div><br></div><div>-Andy</div></div><br><div><br></div></body></html>