[PATCH] D83088: Introduce CfgTraits abstraction

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:133
+      ++m_def;
+      if (m_def == m_instr->defs().end()) {
+        ++m_instr;
----------------
!= return early?


================
Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:136-138
+          // Prefer to avoid support for bundled instructions as long as we
+          // don't really need it.
+          assert(!m_instr->isBundle());
----------------
I've been thinking about more aggressively using bundles around call sites to handle waterfall looping around divergent calls with SGPR arguments


================
Comment at: llvm/lib/CodeGen/MachineCfgTraits.cpp:27-29
+void MachineCfgTraits::Printer::printBlockName(raw_ostream &out,
+                                               MachineBasicBlock *block) const {
+  out << "bb." << block->getNumber();
----------------
I think this should be added to MachineBasicBlock. The same logic is already repeated in MIRPrinter (and the MBB dump function uses a different prefix)


================
Comment at: llvm/lib/CodeGen/MachineCfgTraits.cpp:32
+    if (irBlock->hasName())
+      out << "." << irBlock->getName();
+  }
----------------
Single quotes around .



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088