[PATCH] D82363: [DebugInfo] Add new instruction and expression operator for variadic debug values

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:492
+
   /// Returns a pointer to the operand corresponding to a debug use of Reg, or
   /// nullptr if Reg is not used in any debug operand.
----------------
` \p Reg` ?


================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:503
+  }
+  ArrayRef<MachineOperand *> getDebugOperandsForReg(Register Reg) {
+    assert(isDebugValue() && "Tried to get debug operands for non-debug_value");
----------------
Can we use templates to avoid duplicated code here for `getDebugOperandsForReg()`?


================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:1165
+  }
+  bool isVariadicDebugValue() const {
+    return getOpcode() == TargetOpcode::DBG_VALUE_LIST;
----------------
Should the name of the method follow the new name of the meta instr ?


================
Comment at: llvm/lib/CodeGen/MachineInstr.cpp:2234
   // Propagate Reg to debug value instructions.
-  for (auto *DBI : DbgValues)
-    DBI->getDebugOperandForReg(DefReg)->setReg(Reg);
+  for (auto *DBI : DbgValues) {
+    for (MachineOperand *Op : DBI->getDebugOperandsForReg(DefReg)) {
----------------
No need for the extra curly brackets


================
Comment at: llvm/test/CodeGen/MIR/Generic/dbg-value-list.mir:1
+# RUN: llc -run-pass machineverifier -o - %s | FileCheck %s
+# CHECK: DBG_VALUE_LIST !14, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_stack_value), $edi, $esi, debug-location !15
----------------
Please add a top level comment describing the purpose of the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82363