[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I think it will be worth adding some user level documentation, whether in this patch or another, for the pragma and how autolinking is expecting to work. My apologies I've not had time to go through the code in detail.



================
Comment at: llvm/docs/Extensions.rst:290
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+This section allows for embedding a set of strings specifing libraries to be
----------------
"allows for embedding a set" could probably be worded a bit more simply. For example:
This section contains strings specifying libraries to be added to the link by the linker.


================
Comment at: llvm/docs/LangRef.rst:6084
 
 Some targets support embedding flags to the linker inside individual object
 files. Typically this is used in conjunction with language extensions which
----------------
I prefer the wording of your opening sentence, "Some targets support embedding of" is it worth synchronising?


================
Comment at: llvm/include/llvm/BinaryFormat/ELF.h:841
                                         // for safe ICF.
+  SHT_LLVM_DEPLIBS = 0x6fff4c04,        // LLVM Dependent Library Specifiers.
   // Android's experimental support for SHT_RELR sections.
----------------
This is the same value as has been proposed in another ELF extension in https://reviews.llvm.org/D60242 , May be worth coordinating here. It is probably worth us having a central place to document the LLVM specific ELF extensions as we are accumulating enough of them.


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

https://reviews.llvm.org/D60274