[NFC][LLVM][MI] Refactor MI Printer code.

- Move `MIPrinter` class to anonymous namespace, and remove it as a
  friend of `MachineBasicBlock`.
- Move `canPredictBranchProbabilities` to `MachineBasicBlock` and
  change it to use the new `BranchProbability::normalizeProbabilities`
  function that accepts a range, and also to use llvm::equal() to
  check equality of the two vectors.
- Use `ListSeparator` to print comma separate lists instead of manual
  code to do that. Also added `ListSeparator::reset()` to enable reuse
  of the same ListSeparator object for multiple lists.

Author: jurahul

llvm/include/llvm/ADT/StringExtras.h

@@ -537,6 +537,9 @@ class ListSeparator {
     }
     return Separator;
   }
+
+  // Enables the use of same ListSeparator object for multiple lists.
+  void reset() { First = true; }
 };
 
 /// A forward iterator over partitions of string over a separator.

llvm/include/llvm/CodeGen/MachineBasicBlock.h

@@ -1263,14 +1263,16 @@ class MachineBasicBlock
   /// MachineBranchProbabilityInfo class.
   BranchProbability getSuccProbability(const_succ_iterator Succ) const;
 
+  // Helper function for MIRPrinter.
+  bool canPredictBranchProbabilities() const;
+
 private:
   /// Return probability iterator corresponding to the I successor iterator.
   probability_iterator getProbabilityIterator(succ_iterator I);
   const_probability_iterator
   getProbabilityIterator(const_succ_iterator I) const;
 
   friend class MachineBranchProbabilityInfo;
-  friend class MIPrinter;
 
   // Methods used to maintain doubly linked list of blocks...
   friend struct ilist_callback_traits<MachineBasicBlock>;

llvm/include/llvm/Support/BranchProbability.h

@@ -13,6 +13,7 @@
 #ifndef LLVM_SUPPORT_BRANCHPROBABILITY_H
 #define LLVM_SUPPORT_BRANCHPROBABILITY_H
 
+#include "llvm/ADT/ADL.h"
 #include "llvm/Support/DataTypes.h"
 #include <algorithm>
 #include <cassert>
@@ -62,6 +63,11 @@ class BranchProbability {
   static void normalizeProbabilities(ProbabilityIter Begin,
                                      ProbabilityIter End);
 
+  template <class ProbabilityRange>
+  static void normalizeProbabilities(ProbabilityRange &R) {
+    normalizeProbabilities(adl_begin(R), adl_end(R));
+  }
+
   uint32_t getNumerator() const { return N; }
   static uint32_t getDenominator() { return D; }
 

llvm/lib/CodeGen/MIRPrinter.cpp

@@ -17,6 +17,7 @@
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/CodeGen/MIRYamlMapping.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
@@ -93,10 +94,6 @@ struct FrameIndexOperand {
   }
 };
 
-} // end anonymous namespace
-
-namespace llvm {
-
 /// This class prints out the machine functions using the MIR serialization
 /// format.
 class MIRPrinter {
@@ -151,7 +148,6 @@ class MIPrinter {
   /// Synchronization scope names registered with LLVMContext.
   SmallVector<StringRef, 8> SSNs;
 
-  bool canPredictBranchProbabilities(const MachineBasicBlock &MBB) const;
   bool canPredictSuccessors(const MachineBasicBlock &MBB) const;
 
 public:
@@ -171,10 +167,9 @@ class MIPrinter {
              bool PrintDef = true);
 };
 
-} // end namespace llvm
+} // end anonymous namespace
 
-namespace llvm {
-namespace yaml {
+namespace llvm::yaml {
 
 /// This struct serializes the LLVM IR module.
 template <> struct BlockScalarTraits<Module> {
@@ -188,8 +183,7 @@ template <> struct BlockScalarTraits<Module> {
   }
 };
 
-} // end namespace yaml
-} // end namespace llvm
+} // end namespace llvm::yaml
 
 static void printRegMIR(Register Reg, yaml::StringValue &Dest,
                         const TargetRegisterInfo *TRI) {
@@ -327,9 +321,8 @@ static void printRegFlags(Register Reg,
                           const MachineFunction &MF,
                           const TargetRegisterInfo *TRI) {
   auto FlagValues = TRI->getVRegFlagsOfReg(Reg, MF);
-  for (auto &Flag : FlagValues) {
+  for (auto &Flag : FlagValues)
     RegisterFlags.push_back(yaml::FlowStringValue(Flag.str()));
-  }
 }
 
 void MIRPrinter::convert(yaml::MachineFunction &YamlMF,
@@ -618,9 +611,8 @@ void MIRPrinter::convertCalledGlobals(yaml::MachineFunction &YMF,
   // Sort by position of call instructions.
   llvm::sort(YMF.CalledGlobals.begin(), YMF.CalledGlobals.end(),
              [](yaml::CalledGlobal A, yaml::CalledGlobal B) {
-               if (A.CallSite.BlockNum == B.CallSite.BlockNum)
-                 return A.CallSite.Offset < B.CallSite.Offset;
-               return A.CallSite.BlockNum < B.CallSite.BlockNum;
+               return std::tie(A.CallSite.BlockNum, A.CallSite.Offset) <
+                      std::tie(B.CallSite.BlockNum, B.CallSite.Offset);
              });
 }
 
@@ -630,11 +622,10 @@ void MIRPrinter::convert(yaml::MachineFunction &MF,
   for (const MachineConstantPoolEntry &Constant : ConstantPool.getConstants()) {
     std::string Str;
     raw_string_ostream StrOS(Str);
-    if (Constant.isMachineConstantPoolEntry()) {
+    if (Constant.isMachineConstantPoolEntry())
       Constant.Val.MachineCPVal->print(StrOS);
-    } else {
+    else
       Constant.Val.ConstVal->printAsOperand(StrOS);
-    }
 
     yaml::MachineConstantPoolValue YamlConstant;
     YamlConstant.ID = ID++;
@@ -693,23 +684,6 @@ void llvm::guessSuccessors(const MachineBasicBlock &MBB,
   IsFallthrough = I == MBB.end() || !I->isBarrier();
 }
 
-bool
-MIPrinter::canPredictBranchProbabilities(const MachineBasicBlock &MBB) const {
-  if (MBB.succ_size() <= 1)
-    return true;
-  if (!MBB.hasSuccessorProbabilities())
-    return true;
-
-  SmallVector<BranchProbability,8> Normalized(MBB.Probs.begin(),
-                                              MBB.Probs.end());
-  BranchProbability::normalizeProbabilities(Normalized.begin(),
-                                            Normalized.end());
-  SmallVector<BranchProbability,8> Equal(Normalized.size());
-  BranchProbability::normalizeProbabilities(Equal.begin(), Equal.end());
-
-  return std::equal(Normalized.begin(), Normalized.end(), Equal.begin());
-}
-
 bool MIPrinter::canPredictSuccessors(const MachineBasicBlock &MBB) const {
   SmallVector<MachineBasicBlock*,8> GuessedSuccs;
   bool GuessedFallthrough;
@@ -738,7 +712,7 @@ void MIPrinter::print(const MachineBasicBlock &MBB) {
 
   bool HasLineAttributes = false;
   // Print the successors
-  bool canPredictProbs = canPredictBranchProbabilities(MBB);
+  bool canPredictProbs = MBB.canPredictBranchProbabilities();
   // Even if the list of successors is empty, if we cannot guess it,
   // we need to print it to tell the parser that the list is empty.
   // This is needed, because MI model unreachable as empty blocks
@@ -750,14 +724,12 @@ void MIPrinter::print(const MachineBasicBlock &MBB) {
     OS.indent(2) << "successors:";
     if (!MBB.succ_empty())
       OS << " ";
+    ListSeparator LS;
     for (auto I = MBB.succ_begin(), E = MBB.succ_end(); I != E; ++I) {
-      if (I != MBB.succ_begin())
-        OS << ", ";
-      OS << printMBBReference(**I);
+      OS << LS << printMBBReference(**I);
       if (!SimplifyMIR || !canPredictProbs)
-        OS << '('
-           << format("0x%08" PRIx32, MBB.getSuccProbability(I).getNumerator())
-           << ')';
+        OS << format("(0x%08" PRIx32 ")",
+                     MBB.getSuccProbability(I).getNumerator());
     }
     OS << "\n";
     HasLineAttributes = true;
@@ -768,12 +740,9 @@ void MIPrinter::print(const MachineBasicBlock &MBB) {
   if (!MBB.livein_empty()) {
     const TargetRegisterInfo &TRI = *MRI.getTargetRegisterInfo();
     OS.indent(2) << "liveins: ";
-    bool First = true;
+    ListSeparator LS;
     for (const auto &LI : MBB.liveins_dbg()) {
-      if (!First)
-        OS << ", ";
-      First = false;
-      OS << printReg(LI.PhysReg, &TRI);
+      OS << LS << printReg(LI.PhysReg, &TRI);
       if (!LI.LaneMask.all())
         OS << ":0x" << PrintLaneMask(LI.LaneMask);
     }
@@ -814,18 +783,22 @@ void MIPrinter::print(const MachineInstr &MI) {
 
   SmallBitVector PrintedTypes(8);
   bool ShouldPrintRegisterTies = MI.hasComplexRegisterTies();
-  unsigned I = 0, E = MI.getNumOperands();
-  for (; I < E && MI.getOperand(I).isReg() && MI.getOperand(I).isDef() &&
-         !MI.getOperand(I).isImplicit();
-       ++I) {
-    if (I)
-      OS << ", ";
-    print(MI, I, TRI, TII, ShouldPrintRegisterTies,
-          MI.getTypeToPrint(I, PrintedTypes, MRI),
+  ListSeparator LS;
+  unsigned NumDefs = MI.getNumExplicitDefs();
+
+  // For PATCHPOINT, the output register is optional. Also see
+  // MachineVerifier::visitMachineOperand which does a similar fixup.
+  if (NumDefs != 0 && MI.getOpcode() == TargetOpcode::PATCHPOINT)
+    NumDefs = MI.getOperand(0).isReg() ? NumDefs : 0;
+
+  for (unsigned Idx : llvm::seq(NumDefs)) {
+    OS << LS;
+    print(MI, Idx, TRI, TII, ShouldPrintRegisterTies,
+          MI.getTypeToPrint(Idx, PrintedTypes, MRI),
           /*PrintDef=*/false);
   }
 
-  if (I)
+  if (NumDefs != 0)
     OS << " = ";
   if (MI.getFlag(MachineInstr::FrameSetup))
     OS << "frame-setup ";
@@ -869,74 +842,51 @@ void MIPrinter::print(const MachineInstr &MI) {
     OS << "samesign ";
 
   OS << TII->getName(MI.getOpcode());
-  if (I < E)
-    OS << ' ';
 
-  bool NeedComma = false;
-  for (; I < E; ++I) {
-    if (NeedComma)
-      OS << ", ";
-    print(MI, I, TRI, TII, ShouldPrintRegisterTies,
-          MI.getTypeToPrint(I, PrintedTypes, MRI));
-    NeedComma = true;
+  const unsigned NumOperands = MI.getNumOperands();
+  const unsigned NumUses = NumOperands - NumDefs;
+  LS.reset();
+
+  if (NumUses != 0) {
+    OS << ' ';
+    for (unsigned Idx : llvm::seq<unsigned>(NumDefs, NumOperands)) {
+      OS << LS;
+      print(MI, Idx, TRI, TII, ShouldPrintRegisterTies,
+            MI.getTypeToPrint(Idx, PrintedTypes, MRI));
+    }
   }
 
   // Print any optional symbols attached to this instruction as-if they were
   // operands.
   if (MCSymbol *PreInstrSymbol = MI.getPreInstrSymbol()) {
-    if (NeedComma)
-      OS << ',';
-    OS << " pre-instr-symbol ";
+    OS << LS << " pre-instr-symbol ";
     MachineOperand::printSymbol(OS, *PreInstrSymbol);
-    NeedComma = true;
   }
   if (MCSymbol *PostInstrSymbol = MI.getPostInstrSymbol()) {
-    if (NeedComma)
-      OS << ',';
-    OS << " post-instr-symbol ";
+    OS << LS << " post-instr-symbol ";
     MachineOperand::printSymbol(OS, *PostInstrSymbol);
-    NeedComma = true;
   }
   if (MDNode *HeapAllocMarker = MI.getHeapAllocMarker()) {
-    if (NeedComma)
-      OS << ',';
-    OS << " heap-alloc-marker ";
+    OS << LS << " heap-alloc-marker ";
     HeapAllocMarker->printAsOperand(OS, MST);
-    NeedComma = true;
   }
   if (MDNode *PCSections = MI.getPCSections()) {
-    if (NeedComma)
-      OS << ',';
-    OS << " pcsections ";
+    OS << LS << " pcsections ";
     PCSections->printAsOperand(OS, MST);
-    NeedComma = true;
   }
   if (MDNode *MMRA = MI.getMMRAMetadata()) {
-    if (NeedComma)
-      OS << ',';
-    OS << " mmra ";
+    OS << LS << " mmra ";
     MMRA->printAsOperand(OS, MST);
-    NeedComma = true;
-  }
-  if (uint32_t CFIType = MI.getCFIType()) {
-    if (NeedComma)
-      OS << ',';
-    OS << " cfi-type " << CFIType;
-    NeedComma = true;
   }
+  if (uint32_t CFIType = MI.getCFIType())
+    OS << LS << " cfi-type " << CFIType;
 
-  if (auto Num = MI.peekDebugInstrNum()) {
-    if (NeedComma)
-      OS << ',';
-    OS << " debug-instr-number " << Num;
-    NeedComma = true;
-  }
+  if (auto Num = MI.peekDebugInstrNum())
+    OS << LS << " debug-instr-number " << Num;
 
   if (PrintLocations) {
     if (const DebugLoc &DL = MI.getDebugLoc()) {
-      if (NeedComma)
-        OS << ',';
-      OS << " debug-location ";
+      OS << LS << " debug-location ";
       DL->printAsOperand(OS, MST);
     }
   }
@@ -945,12 +895,10 @@ void MIPrinter::print(const MachineInstr &MI) {
     OS << " :: ";
     const LLVMContext &Context = MF->getFunction().getContext();
     const MachineFrameInfo &MFI = MF->getFrameInfo();
-    bool NeedComma = false;
+    LS.reset();
     for (const auto *Op : MI.memoperands()) {
-      if (NeedComma)
-        OS << ", ";
+      OS << LS;
       Op->print(OS, MST, SSNs, Context, &MFI, TII);
-      NeedComma = true;
     }
   }
 }

llvm/lib/CodeGen/MIRPrintingPass.cpp

@@ -81,10 +81,6 @@ char MIRPrintingPass::ID = 0;
 char &llvm::MIRPrintingPassID = MIRPrintingPass::ID;
 INITIALIZE_PASS(MIRPrintingPass, "mir-printer", "MIR Printer", false, false)
 
-namespace llvm {
-
-MachineFunctionPass *createPrintMIRPass(raw_ostream &OS) {
+MachineFunctionPass *llvm::createPrintMIRPass(raw_ostream &OS) {
   return new MIRPrintingPass(OS);
 }
-
-} // end namespace llvm