[PATCH] D102808: [NFC][SampleFDO] Add more debugging logic.

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h:217
 #ifndef NDEBUG
+template <typename BT> static inline std::string getName(const BT *BB) {
+  if (BB->hasName())
----------------
This is useful, I wondering if this can be used to improve print block names in general, in addition to sample loader.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:889-890
     if (DI) {
+      LLVM_DEBUG(dbgs() << "Promoted indirect call " << CI << "\n"
+                        << " to " << *DI << "\n");
       Sum -= Candidate.CallsiteCount;
----------------
Inside `pgo::promoteIndirectCall`, there is a remark covering this info already under `pgo-icall-prom`.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1178
               << "incompatible inlining");
+    LLVM_DEBUG(dbgs() << "Failed to inline call " << CB
+                      << "\n because of incompatible inlining\n");
----------------
This kind of message looks like the one that should be covered by remarks, and it's covered here. Is the remarks not enough? Looking at other place (Inliner/InlineCost, etc), we don't duplicate those messages between remarks and LLVM_DEBUG.

Same for the ones below.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1268
   *NewCandidate = {CB, CalleeSamples, CallsiteCount, Factor};
+  LLVM_DEBUG(dbgs() << "Inline Candidate: " << *CB << "\n");
   return true;
----------------
This seems a bit random/lack of structure. There's nothing marking the debugging message's start/end for a functions.

If we want, we can organize the messages so we know which function/inliner each message is about.

Also a general comment, adding all debug prints used when debugging smaller programs would make the debugging message hard to use for large programs. So I'd be a bit cautious about adding those. 


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1433
   if (!CQueue.empty()) {
+    LLVM_DEBUG(dbgs() << "Inlining stopped, out of budget\n");
     if (SizeLimit == (unsigned)ProfileInlineLimitMax)
----------------
Unless the messages are structured, i.e there's a paired message saying which function we're currently processing, it'd be good to point out the function/inliner too. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102808