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

[Jeremy Morse via Phabricator via llvm-commits]

jmorse marked 3 inline comments as done.
jmorse added inline comments.


================
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]) {
----------------
CarlosAlbertoEnciso wrote:
> 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);
> ```
> 
I don't know that there's any useful abstraction occurring there -- once you've called MaybeDead->getOperand(1) (which is copy-instruction specific) the code ceases to be about any particular instruction. Possibly it could be a static MachineInstr function?

IMHO this is a direction to be avoided though: collectDebugValues gives the impression (via its name) that it's a function that works in the general case, but (IMHO again) all the users really have context-specific requirements. Repeating some code in these circumstances is useful to highlight the point that there's no easy (or general) solution.


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

https://reviews.llvm.org/D56265