<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 8, 2016 at 3:05 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">anemet added a comment.<br>
<span class=""><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>
> There are already too many classes related to BFI, so I am not a fan of introducing another LazyBFI.<br>
><br>
> 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>
</span>Moving it to MBFI, SGTM but I don't think that forcing the calculation of the frequencies in getBlockFreq is sufficient.<br>
<br>
So just to be clear, you're suggesting sprinkling around in the code like this:<br>
<br>
  if (!Calculated)<br>
    calculate()<br>
<br>
in getBlockFreq, getBlockProfileCount, setBlockFreq, getFloatingBlockFreq, print and printBlockFreq?<br>
<br></blockquote><div><br></div><div>getBlockProfileCount will call getBlockFreq, so there is no need to add check there.</div><div><br></div><div>setBlockFreq is an incremental update interface. We can assert it is already calculated so there is no need to add it either.</div><div><br></div><div>printBlockFreq calls getFloatingBlockFreq, so there is no need to do check there either.</div><div><br></div><div>This leaves only two places:</div><div><br></div><div>getBlockFreq and getFloatingBlockFreq. There might be a way to refactor these two, but I am not too concerned about slight duplication here.</div><div><br></div><div>David</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<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></div>