<div dir="ltr">The idea of lazyBFI analysis sounds good to me. Need to see what the actual patch look like :)<div><br></div><div>David</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 11, 2016 at 2:18 PM, Adam Nemet <span dir="ltr"><<a href="mailto:anemet@apple.com" target="_blank">anemet@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">anemet added a comment.<br>
<br>
> In <a href="http://reviews.llvm.org/D22141#478534" rel="noreferrer" target="_blank">http://reviews.llvm.org/D22141#478534</a>, @davidxl wrote:<br>
<br>
><br>
<br>
</span><span class="">> > I suggest just making BFI lazy itself (when getBlockFreq() is first called ...). In fact  I think the laziness handling should be in BlocFrequencyInfoImpl class. By so doing, MBFI can get the laziness automatically as well.<br>
<br>
><br>
<br>
<br>
</span>I've tried and unfortunately, I don't think this will work.  I saw two issues:<br>
<br>
a. LoopInfo is freed by the time block frequencies are requested<br>
<br>
  This can be fixed by making users of BFI also dependent on LI but that's an overkill for existing users<br>
<br>
b. The pass starts modifying the CFG and when it tries to update the frequencies LI and the CFG are already inconsistent<br>
<br>
I think that these issues (there maybe more) are difficult to fix in the context of existing BFI's users.  My new proposal is to introduce a new analysis, LazyBFI.  Then, new users could opt in by complying with LBFI's requirements:<br>
<br>
1. require LI and BPI as well (I will add an 'required' enumerator)<br>
<br>
2. compute BFI before making any CFG changes<br>
<br>
The analysis would effectively consist of the LazyBFI class I had earlier.  It will only cover IR BFI for now but we could later template-ize it to also work with BFI.<br>
<br>
I talked with Duncan about this and he was fine with this idea.  Let me know your thoughts.<br>
<br>
Adam<br>
<br>
<br>
<a href="http://reviews.llvm.org/D22141" rel="noreferrer" target="_blank">http://reviews.llvm.org/D22141</a><br>
<br>
<br>
<br>
</blockquote></div><br></div>