This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][NFC] Heterogeneous DWARF extensions update
ClosedPublic

Authored by t-tye on Jan 11 2023, 2:42 PM.

Details

Summary
  • Clarify CFI rules in heterogeneous DWARF extensions
  • Added DWARF source language memory spaces.
  • Added DWARF architecture address spaces.
  • Other minor corrections.

Diff Detail

Event Timeline

t-tye created this revision.Jan 11 2023, 2:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 11 2023, 2:42 PM
t-tye requested review of this revision.Jan 11 2023, 2:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 11 2023, 2:42 PM
t-tye updated this revision to Diff 488420.Jan 11 2023, 4:26 PM

Add common block DW_AT_location description.

LGTM save a few nits/questions

llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst
3612

This "attribute" is outside of the code-backticks in other places, did you intend to put it inside here?

3722

This comma seems extraneous

4702

I assume this is saying "the rest of the existing DWARF5 text as-is"?

4704

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)?

4897

Is the second column required for this to parse, or to match the two-column layout in the spec?

t-tye updated this revision to Diff 488813.Jan 12 2023, 4:54 PM
t-tye marked an inline comment as done.

Fix review comments.

t-tye marked an inline comment as done.Jan 12 2023, 4:55 PM
t-tye added inline comments.
llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst
3612

Fixed.

3722

Fixed.

4702

Correct.

4704

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.

4897

It was the only way I found to stop the parser complaining about a single table being expected.

scott.linder accepted this revision.Jan 12 2023, 5:27 PM

LGTM, thank you!

llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst
4704

OK, I see now, thank you for clarifying!

4897

Makes sense, I assumed as much, just making sure it wasn't an oversight

This revision is now accepted and ready to land.Jan 12 2023, 5:27 PM
This revision was landed with ongoing or failed builds.Jan 12 2023, 6:08 PM
This revision was automatically updated to reflect the committed changes.
t-tye marked an inline comment as done.