[PATCH] D83495: [DebugInfo] Add DWARF emission for DBG_VALUE_LIST

[Scott Linder via Phabricator via llvm-commits]

scott.linder added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:952-973
+    case MachineOperand::MO_Register:
+    case MachineOperand::MO_FrameIndex: {
+      Register Reg;
+      if (Op.isReg())
+        Reg = Op.getReg();
+      else {
+        const TargetFrameLowering *TFI =
----------------
StephenTozer wrote:
> scott.linder wrote:
> > If you are changing this anyway, I'd vote to just split up these two cases, they don't actually seem related and the existing code reads worse to me.
> The similarity here is that even when `Op` is a register, it can still have an offset - it's just calculated above, outside of this loop, by the debug offset operand of `MI`. I do think this looks quite messy; it'd probably be better to move the offset calculation down into this loop, so that it can be understood for both of them as a local variable.
+1

I think I completely missed the connection to the offset calculated above, thank you for clarifying.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83495