[PATCH] D22694: [Inliner, OptDiag] Add hotness attribute to opt diagnostics
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 8 23:25:32 PDT 2016
chandlerc added a subscriber: chandlerc.
chandlerc accepted this revision.
chandlerc added a reviewer: chandlerc.
chandlerc added a comment.
This revision is now accepted and ready to land.
LGTM w.r.t. to the inliner. Please make sure the other heavy users of remarks are happy as well as you are changing the output mechanism here.
I agree with the sentiment in the patch description that this is a bit gross, but at least seems sufficiently isolated.
================
Comment at: include/llvm/Analysis/OptimizationDiagnosticInfo.h:37-38
@@ -36,1 +36,4 @@
+ // This variant can be used to generate ORE on demand (without the analysis
+ // pass).
+ OptimizationRemarkEmitter(Function *F);
----------------
This needs a proper doxygen comment, especially regarding the cost of doing this. And doubly especially that it is free unless the context has the hotness emission specifically enabled. I had to read the implementation to understand why this is OK. =[
I'll also note that this applies to this entire file which seems ... remarkably devoid of doxygen API comments. Please document this interface and infrastructure in subsequent commits. =/
================
Comment at: include/llvm/Analysis/OptimizationDiagnosticInfo.h:156
@@ -151,1 +155,3 @@
+ /// \brief If we generate BFI on demand, we need to free it when ORE is freed.
+ std::unique_ptr<BlockFrequencyInfo> OwnedBFI;
----------------
No need for '\brief'.
https://reviews.llvm.org/D22694
More information about the llvm-commits
mailing list