<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jul 12, 2016 at 3:56 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 inline comments.<br>
<span class=""><br>
================<br>
Comment at: include/llvm/Analysis/LazyBlockFrequencyInfo.h:22<br>
@@ +21,3 @@<br>
+//<br>
+// 2. Similarly, getAnalysisUsage should call:<br>
+//<br>
----------------<br>
</span><span class="">davidxl wrote:<br>
> What is the use model for the new pass manager? Anything worth documenting?<br>
</span>I am not sure we will need this class with the new PM.  Correct me if I am wrong but I thought as long as the client does not call something like getResult<BlockFrequencyAnalysis>(F), the analysis won't be populated.  This is not the case for the old PM.<br>
<br>
I was going to play with this after this patch and post follow-on patches if anything is required for the new PM.<br>
<br>
Since the vectorizer is converted now, I should be able to try it there.<br></blockquote><div><br></div><div><br></div><div>ok.  Perhaps add a comment there so that people don't start porting it by accident.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
================<br>
Comment at: include/llvm/Analysis/LazyBlockFrequencyInfo.h:91<br>
@@ +90,3 @@<br>
+/// the file comment for the rules of how to use this properly.<br>
+class LazyBlockFrequencyInfoPass : public FunctionPass {<br>
+  LazyBlockFrequencyInfo LBFI;<br>
----------------<br>
</span><span class="">davidxl wrote:<br>
> Can the wrapper class of BFI be 'inlined' into this class so that we don't need LBFI ?<br>
</span>It could certainly live nested in the pass, I would rather not remove it though; it nicely captures the single functionality it provides and the abstraction overhead should be removed by the compiler.<br></blockquote><div><br></div><div><br></div><div>If new PM port is needed, I see the point of having LBFI to be shared. If not -- i.e., it is only used by the WrapperPass, I don't see the need for it -- the patch will also be smaller. This is not an issue about overhead (which has none). As I mentioned before, the issue is the bloated number of classes related to BFI -- so removing this unnecessary abstraction has value for maintenance reasons.</div><div> </div><div><br></div><div>David</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<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></div>