[PATCH] D71501: Cost Annotation Writer for InlineCost

[Artur Pilipenko via Phabricator via llvm-commits]

apilipenko accepted this revision.
apilipenko added a comment.
This revision is now accepted and ready to land.

Couple of nit comments inlined, LGTM once addressed.



================
Comment at: llvm/lib/Analysis/InlineCost.cpp:56
 
+static cl::opt<bool> EnableInstructionDelta("print-instruction-delta",
+    cl::Hidden, cl::init(false),
----------------
Suggest to rename the global and the flag to PrintDebugInstructionDeltas("print-instruction-deltas", ...)


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:124-125
+
+  virtual void emitInstructionAnnot(const llvm::Instruction *I,
+                                    llvm::formatted_raw_ostream &OS);
+};
----------------
There are still redundant llvm:: prefixes.


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:1838-1839
     // consumed the instruction in any way, and false if the instruction's base
-    // cost should count against inlining.
+    // cost should count against inlining. The deltas are saved for the
+    // CostAnnotationWriter.
+    onInstructionAnalysisStart(&*I);
----------------
This comment is specific to InlineCostCallAnalyzer and doesn't belong here. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71501/new/

https://reviews.llvm.org/D71501