[PATCH] D145076: [llvm][DebugInfo] Introduce new DW_AT_LLVM_preferred_name attribute

[Adrian Prantl via Phabricator via llvm-commits]

aprantl requested changes to this revision.
aprantl added a comment.
This revision now requires changes to proceed.

Looks pretty good, some nitpicks inside.



================
Comment at: llvm/include/llvm/BinaryFormat/Dwarf.def:609
 HANDLE_DW_AT(0x3e09, LLVM_ptrauth_authenticates_null_values, 0, LLVM)
+HANDLE_DW_AT(0x3e0a, LLVM_preferred_name, 0, APPLE)
 
----------------
This should be an LLVM attribute. There's no reason why it wouldn't work on FreeBSD which also uses clang+lldb, right?

Can you document this attribute in LangRef.rst or SourceLevelDebugging.rst?


================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:1304
+
+  void replacePreferredName(Metadata *PreferredName) {
+    replaceOperandWith(14, PreferredName);
----------------
Is this actually needed?


================
Comment at: llvm/test/Bitcode/attr-preferred_name-dicomposite.ll:1
+; REQUIRES: x86-registered-target
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
----------------
this shouldn't be needed.


================
Comment at: llvm/test/Bitcode/attr-preferred_name-dicomposite.ll:2
+; REQUIRES: x86-registered-target
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+
----------------
I think this should be in the IR subdir and do a double-roundtrip (as|dis|as|dis). See the other tests for examples.


================
Comment at: llvm/test/Bitcode/attr-preferred_name-dicomposite.ll:4
+
+%struct.Foo = type { i8 }
+
----------------
I think we also want a test/DebugInfo test that checks dwarfdump output?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145076