[PATCH] D141548: [AMDGPU][NFC] Heterogeneous DWARF extensions update

[Tony Tye via Phabricator via llvm-commits]

t-tye marked an inline comment as done.
t-tye added inline comments.


================
Comment at: llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst:3612
+
+1.  A ``DW_AT_location attribute``, whose value is a DWARF expression E that
+    describes the location of a variable or parameter at run-time.
----------------
scott.linder wrote:
> This "attribute" is outside of the code-backticks in other places, did you intend to put it inside here?
Fixed.


================
Comment at: llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst:3722
+
+  This section, augments DWARF Version 5 section 5.3.
+
----------------
scott.linder wrote:
> This comma seems extraneous
Fixed.


================
Comment at: llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst:4702
+
+      - The first type of reference...
+
----------------
scott.linder wrote:
> I assume this is saying "the rest of the existing DWARF5 text as-is"?
Correct.


================
Comment at: llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst:4704
+
+      - The second type of reference can identify any debugging information
+        entry within a .debug_info section; in particular, it may refer to an
----------------
scott.linder wrote:
> This is just removing the text "For references from one shared object or static executable file to another, the relocation and identification of the target object must be performed by the consumer", right? Is this because it is redundant (and covered by the non-normative notes)?
DWARF does not provide any means to identify a reference from one linked object to another. For all practical purposes, DW_OP_call_ref, DW_OP_implicit_pointer, and DW_FORM_ref_addr can only be used to refer to an offset within the same executable or shared object. Their purpose is for references between compilation units, not between linked objects. Therefore mention of the consumer resolving the reference to another executable/shared object has been removed from all three places.


================
Comment at: llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst:4897
+
+   ================================== =======
+   ``DW_AT_LLVM_address_space``
----------------
scott.linder wrote:
> Is the second column required for this to parse, or to match the two-column layout in the spec?
It was the only way I found to stop the parser complaining about a single table being expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141548