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

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.
Herald added subscribers: dexonsmith, pengfei.

@StephenTozer what's the status of the latest discussions on this patch? As far as I understand it:

- DBG_VALUE may go away, but that'll be in another patch,
- There was some conclusion to whether all DBG_VALUE_LISTs should be stack_values too, can't remember what, on the mailing list,
- This patch, adding the plumbing for the DBG_VALUE_LIST, would be ready to land.

For the record, it LGTMs. I've added some mega-nits inline that are largely style based.



================
Comment at: llvm/docs/SourceLevelDebugging.rst:608
+ * Operand 2 is the Expression field of the original debug intrinsic.
+ * Any number of operands, from the 3rd onwards, record a set of variable
+   location operands, which may take any of the same values as the first
----------------
Gratuitous nit -- it's a sequence rather than a set


================
Comment at: llvm/lib/CodeGen/MachineInstr.cpp:2193
+  NewMI.addMetadata(Orig.getDebugVariable()).addMetadata(Expr);
+  if (Orig.isDebugValueList())
+    for (const MachineOperand &Op : Orig.debug_operands())
----------------
Braces to avoid ambiguity with the contained blocks?


================
Comment at: llvm/lib/Target/X86/X86OptimizeLEAs.cpp:610-615
+    NewOps.push_back(Op.isReg() && Op.getReg() == OldReg
+                         ? MachineOperand::CreateReg(NewReg, false, false,
+                                                     false, false, false, false,
+                                                     0,
+                                                     /*IsRenamable*/ true)
+                         : Op);
----------------
I'd suggest lambda-ising this for readability


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82363