[PATCH] D56265: [DebugInfo] MCP: collect and update DBG_VALUEs encountered in local block

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:222
 
+  /// Copy -> DbgUsers multimap.
+  DenseMap<MachineInstr*, TinyPtrVector<MachineInstr*>> CopyDbgUsers;
----------------
May be a more descriptive comment; something like:

/// Keeps track of debug users for the current basic block.


================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:246
     if (MachineInstr *Copy = Tracker.findCopyForUnit(*RUI, *TRI)) {
-      LLVM_DEBUG(dbgs() << "MCP: Copy is used - not dead: "; Copy->dump());
-      MaybeDeadCopies.remove(Copy);
+      if (!IsDebug) {
+        LLVM_DEBUG(dbgs() << "MCP: Copy is used - not dead: "; Copy->dump());
----------------
May be change the logic to:

```if (IsDebug)
  CopyDbgUsers[Copy].push_back(&Reader);
else {
  LLVM_DEBUG(dbgs() << "MCP: Copy is used - not dead: "; Copy->dump());
  MaybeDeadCopies.remove(Copy);
}```




================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:625
       assert(MaybeDead->isCopy());
-      MaybeDead->changeDebugValuesDefReg(MaybeDead->getOperand(1).getReg());
+      unsigned SrcReg = MaybeDead->getOperand(1).getReg();
+      for (auto DbgUser : CopyDbgUsers[MaybeDead]) {
----------------
As your intention is to replace the calls (direct and indirect) to `collectDebugValues` and `changeDebugValuesDefReg`, why not move the new code to a function that can be reused.

```
MaybeDead->someNewFunction(MaybeDead->getOperand(1).getReg(), CopyDbgUsers);
```



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

https://reviews.llvm.org/D56265