[PATCH] D158889: [AsmPrinter][PGO] Adds optional dumping of branch probabilities for PGO metrics.

[Micah Weston via Phabricator via llvm-commits]

red1bluelost marked 2 inline comments as done.
red1bluelost added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1657
   bool CanDoExtraAnalysis = ORE->allowExtraAnalysis(DEBUG_TYPE);
+  const bool CanDumpBranchProbs = EnableBranchProbabilityDumping;
   for (auto &MBB : *MF) {
----------------
mtrofin wrote:
> why not just use `EnableBranchProbabilityDumping`?
Since we are reading from a global variable and the following loop it probably hot code, I thought it would be better to cache it outside the loop rather than keep referencing the global.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1760
       default:
+        if (CanDumpBranchProbs && MI.isConditionalBranch() && !MI.isCall()) {
+          if (auto *MBPI =
----------------
mtrofin wrote:
> what about virtual calls?
I don't think MachineInstr has a notion of virtual calls. Unless you mean something else or I'm missing something.


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

https://reviews.llvm.org/D158889