[PATCH] D143311: [MLGO] Add BB Profile Dump Pass for Regalloc Case

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added a comment.

You'll need to update the failing pass structure tests, when you do, can you put a comment there (in the tests) that this pass must be right before asmprinter to make sure nothing mutates MBB freqs?



================
Comment at: llvm/include/llvm/CodeGen/Passes.h:582
+  // When learning an eviction policy, export basic blocks and profile info
+  FunctionPass *createRegAllocBBProfileDumpPass();
+
----------------
It's not really regalloc-tied, maybe `createFinalProfileDumpPass` (so "FinalProfileDump" being the pass)?


================
Comment at: llvm/lib/CodeGen/MLRegallocEvictAdvisor.cpp:56
+static cl::opt<std::string> BasicBlockProfileDump(
+    "regalloc-bb-profile-dump", cl::Hidden,
+    cl::desc("Basic block profile dump for external cost modelling"));
----------------
not `regalloc`. Maybe `mbb-profile-dump`?

Also, can the comment say that matching this info with BBs afterwards, using e.g. `llvm-objdump --symbolize-operands`, requires the compilation be performed with `-fbasic-block-sections=labels`?


================
Comment at: llvm/lib/CodeGen/MLRegallocEvictAdvisor.cpp:122
 
+// The BB frequency/mc dump pass.
+// This dumps the basic blocks along with some frequency info right before
----------------
needs its own .cpp, it's general-purpose


================
Comment at: llvm/lib/CodeGen/MLRegallocEvictAdvisor.cpp:1190
+      *FileOutput.get() << MF.getName() << ","
+                        << MBB.getNumber() << ","
+                        << MBFI.getBlockFreqRelativeToEntryBlock(&MBB) << "\n";
----------------
@rahmanl this is where we wanna make sure we get it right: is this the right API that would ensure the output obtained here (so right before AsmPrint) matches the BB #s you get if you `-fbasic-block-sections=lables` and then `llvm-objdump --symbolize-operands` - I realize the latter outputs `BB<nr>`, but other than that (i.e. ignoring the "BB" prefix)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143311