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

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer marked 2 inline comments as done.
StephenTozer added a comment.

In D83495#2158601 <https://reviews.llvm.org/D83495#2158601>, @aprantl wrote:

> Could we reduce complexity by entirely replacing DBG_VALUE with DBG_VALUE_LIST, *, DW_OP_arg 0, *?


Yes, and in the long run I think that should be the goal; it would also be nice to remove the "indirect" operand and all the code paths that use it.  So far it has been easier to create this as a separate instruction, but if the debug info cabal as a whole is positive on the replacement then there's no harm in bringing the replacement into these patches (or more likely, adding an extra patch to do so on top of the ones that are already being reviewed).



================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:904
+      OS << ", ";
+    if (Op.isFPImm()) {
+      APFloat APF = APFloat(Op.getFPImm()->getValueAPF());
----------------
aprantl wrote:
> Out of curiosity: Is there an operand kind that we could switch() over?
It would be useful to have one if there isn't; I didn't want to fold a change like that into this work, but if it exists I can use it (and if not it'd be nice to add in another patch).


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:435
+    case dwarf::DW_OP_LLVM_arg:
+      if (!InsertArg(Op->getArg(0), ExprCursor)) {
+        LocationKind = Unknown;
----------------
aprantl wrote:
> By using a callback here the callee cannot use the advanced functionality of addMachineRegExpression for any but a leading DW_OP_LLVM_arg. Do you see a way of either generalizing addMachineRegExpression or otherwise reorganizing this so the addMachineRegExpression functionality becomes available to DBG_VALUE_LIST?
It actually can use that functionality - in this case, all of the functionality that would normally be applied to the location in the DBG_VALUE is applied by this callback. The callback in this case can advance the ExprCursor, so there are no issues with using addMachineRegExpression normally at any point in a DBG_VALUE_LIST.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83495