[PATCH] D123394: [CodeGen] Late cleanup of redundant address/immediate definitions.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:1492-1495
+  while (Change) {
+    Change = false;
+    for (auto &MBB : Worklist)
+      if (!Visited.count(MBB) && allPredsVisited(MBB)) {
----------------
Can't you use one of the existing block visiting iterators?


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:1514
+// Slow version for now: Update live-in lists and clear kill flags after each
+// removal of a redundant definition. Not sure how important kill-flags are
+// after PEI, but this clears them carefully on each user. TODO: This should
----------------
If PEI's loop was reversed and scavenge register backwards was used, we wouldn't need kill flags at all


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:1629
+      LLVM_DEBUG(dbgs() << "Removing redundant instruction in MBB#"
+                 << MBB->getNumber() << ":";  MI->dump(););
+      removeRedundantDef(MI, TRI);
----------------
*MI works, don't need dump


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:1643
+      LLVM_DEBUG(dbgs() << "Found interesting instruction in MBB#"
+                 << MBB->getNumber() << ":";  MI->dump(););
+      MBBDefs[DefedReg] = MI;
----------------
Ditto


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:135-136
+  // Data structures to map regs to definitions per MBB.
+  typedef std::map<Register, MachineInstr*> Reg2DefMap;
+  typedef std::map<MachineBasicBlock *, Reg2DefMap> MBB2RegDefsMap;
+  void visitBlock(MachineBasicBlock *MBB,
----------------
DenseMap?


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:138-139
+  void visitBlock(MachineBasicBlock *MBB,
+                  std::set<MachineBasicBlock *> &Visited,
+                  std::list<MachineBasicBlock *> &Worklist,
+                  MBB2RegDefsMap &RegDefs);
----------------
SmallSetVector?


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

https://reviews.llvm.org/D123394