[PATCH] D41278: [MachineCombiner] Improve debug output (NFC)

[Andrew V. Tischenko via Phabricator via llvm-commits]

avt77 added inline comments.


================
Comment at: lib/CodeGen/MachineCombiner.cpp:432
   DEBUG(dbgs() << "Combining MBB " << MBB->getName() << "\n");
-
+#ifndef NDEBUG
+  const TargetSubtargetInfo &STI = MF->getSubtarget();
----------------
Gerolf wrote:
> + 1 
> 
> Also, why recompute STI?
It seems it's better to use *STI initialized in runOnFuction - I'll change it.


================
Comment at: lib/CodeGen/MachineCombiner.cpp:478
 
+    DEBUG(dbgs() << " Possible instr(s) to replace\n");
+    DEBUG(dbgs() << "\t" << STI.getSchedInfoStr(MI) << ": ";
----------------
Gerolf wrote:
> Duplicate info to what is displayed in the loops below?
No, here we have only one possible instr to remove (the message should be changed). Below we could have a set of instrs to remove.


================
Comment at: lib/CodeGen/MachineCombiner.cpp:496
 
+      DEBUG(dbgs() << "\tThese instructions could be removed\n");
+      for (auto const *InstrPtr : DelInstrs) {
----------------
Gerolf wrote:
> The loops should be wrapped in DEBUG. Since this is a lot of info that is potentially logged I suggest to offer an option to enable this dump.
OK, I'll create such an option.


================
Comment at: lib/CodeGen/MachineCombiner.cpp:567
 
-bool MachineCombiner::runOnMachineFunction(MachineFunction &MF) {
-  const TargetSubtargetInfo &STI = MF.getSubtarget();
+bool MachineCombiner::runOnMachineFunction(MachineFunction &_MF) {
+  MF = &_MF;
----------------
Gerolf wrote:
> What motivates the changes in this function?
I'd like to store MF here to be able to calculate STI reference later but it seems I should store *STI here instead of that and use this pointer instead of reference..


https://reviews.llvm.org/D41278