[PATCH] D105217: [LLD] Adding support for RELA for CG Profile.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: lld/test/ELF/cgprofile-rela.test:1
+## GNU tools strip/objcopy change REL to RELA. Testing the RELA path.
+# REQUIRES: x86
----------------
I'd rewrite the comment like this:

"Under some circumstances, GNU tools strip/objcopy change REL to RELA. Test that LLD can handle call graph profile data relocated with RELA relocations."


================
Comment at: lld/test/ELF/cgprofile-rela.test:32
+    Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
+    Content: 0000
+  - Name: .text.C
----------------
I assume your aim is for 2-byte sections? If so, you can replace these lines with `Size: 2`, which I think expresses the intent better.


================
Comment at: lld/test/ELF/cgprofile-rela.test:63
+    Type: SHT_RELA
+    Link: .symtab
+    AddressAlign: 0x8
----------------
I believe the `Link: .symtab` is implicit for relocation sections. You can remove it.


================
Comment at: lld/test/ELF/cgprofile-rela.test:64
+    Link: .symtab
+    AddressAlign: 0x8
+    Info: .llvm.call-graph-profile
----------------
Similarly: I think the address align field has a default value for reloc sections. No need to explicitly specify it.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/call-graph-profile.test:209
 
-## Check we report a warning when a relocation section cant't be loaded.
+## Check we report a warning when a REl relocation section cant't be loaded.
 # RUN: yaml2obj %s --docnum=5 -o %t6.o
----------------
Two typos.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/call-graph-profile.test:307
+
+## Check we report a warning when a RELA relocation section cant't be loaded.
+# RUN: yaml2obj %s --docnum=7 -o %t8.o
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105217