[PATCH] D58042: [LiveDebugValues] Emit parameter's entry value

Mon Apr 15 20:56:29 PDT 2019

aprantl added inline comments.

================
Comment at: include/llvm/IR/DebugInfoMetadata.h:2529
+  enum { NoDeref = false, WithDeref = true, WithStackValue = true,
+         WithEntryValue = true };

----------------
The more flags we add these bool parameters get less and less safe, since the order isn't typesafe. We should probably think about also using an `unsigned Flag` parameter that is `WithDeref | WithEntryValue`.

================
Comment at: include/llvm/IR/DebugInfoMetadata.h:2600
+    return getNumElements() > 0 &&
+           getElement(0) == dwarf::DW_OP_GNU_entry_value;
+  }
----------------
In LLVM IR I'd prefer using the DWARF 5 DW_OP_entry_value (Or a new LLVM one if we change the semantics). We can still emit the GNU variant, if requested.

According to the DWARF spec an entry value can appear anywhere in the dwarf expression, right? If we only accept one as the first element, we also need a verifier that enforces this.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfExpression.cpp:333
+
+  emitOp(dwarf::DW_OP_GNU_entry_value);
+  emitUnsigned(Op->getArg(0));
----------------
This should be DW_OP_entry_value if DWARF 5 is requested (or a perhaps GDB tuning)

================
Comment at: lib/CodeGen/LiveDebugValues.cpp:450

+void LiveDebugValues::emitEntryValues(
+    MachineInstr &MI, OpenRangesSet &OpenRanges, VarLocMap &VarLocIDs,
----------------
Would it be possible to split this part into a separate patch?

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

https://reviews.llvm.org/D58042