[PATCH] D76878: Implement DW_{OP,AT}_LLVM_* for Heterogeneous Debugging

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1056
 
+  if (useHeterogeneousExtensionAttributes()) {
+    NewCU.addString(Die, dwarf::DW_AT_LLVM_augmentation, "[amdgpu:v0.1]");
----------------
no need for the curly brackets here


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h:750
+  /// Returns whether extensions defined at
+  /// https://llvm.org/docs/AMDGPUDwarfProposalForHeterogeneousDebugging.html
+  /// are enabled.
----------------
I'd say that the link was mentioned too many times.


================
Comment at: llvm/lib/MC/MCDwarf.cpp:1662
+  if (MAI->supportsHeterogeneousDebuggingExtensions())
+    Augmentation += "[llvm:v0.0]";
+  Streamer.emitBytes(Augmentation);
----------------
clayborg wrote:
> Should we use a single character here? If we don't, unwinders might end up parsing each character individually and that might make unwinders fail as they parse each character. Or does the unwind spec state that everything between square brackets is a single entry? This seems dangerous unless the spec has some rule like this.
@RamNalamothu have you taken a look into this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76878