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

[Jeremy Morse via Phabricator via llvm-commits]

jmorse marked an inline comment 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]) {
----------------
jmorse wrote:
> 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.
Reading back that doesn't sound great, what I meant was: "Let's not try too hard to de-duplicate when IMHO all the users of collectDebugValues have very context specific requirements"


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

https://reviews.llvm.org/D56265