[PATCH] D153215: [Object] Add ELF section type SHT_LLVM_LTO for fat LTO

Wed Jun 21 01:20:31 PDT 2023

jhenderson added a comment.

In D153215#4435866 <https://reviews.llvm.org/D153215#4435866>, @MaskRay wrote:

> In D153215#4431926 <https://reviews.llvm.org/D153215#4431926>, @jhenderson wrote:
>
>> Is it worth an llvm-objdump test too?
>>
>> Functionally looks fine, but I have no opinion on the overall feature itself (and therefore whether a new section type etc is needed for it).
>>
>> I will say that if the feature has been accepted, a new section type would seem appropriate, so that tooling can handle it appropriately without having to check section names.
>
> I think normal code paths of `llvm-objdump` do not print section types. (`llvm-objdump -h` doesn't).
> If there is an error, `llvm-objdump` will use `llvm::object::describe` to give an error, but the message is indirect. ``llvm::object::describe`` is tested by `llvm-readobj`.

Good point, thanks.

================
Comment at: llvm/include/llvm/BinaryFormat/ELF.h:1039
   SHT_LLVM_OFFLOADING = 0x6fff4c0b,         // LLVM device offloading data.
+  SHT_LLVM_LTO = 0x6fff4c0c,                // .llvm.lto for fat LTO.
   // Android's experimental support for SHT_RELR sections.
----------------
paulkirth wrote:
> jhenderson wrote:
> > Perhaps `SHT_LLVM_FAT_LTO` so that it is a little more self-descriptive? (Equivalent comment applies to the section name and similar bits of data)
> While this is for FatLTO now, I don't see why the section flag needs that distinction, since its a bit more generic (i.e., it's bitcode for LTO whether you use it for FatLTO or not). I don't have super strong feelings here, but I don't see much value in labeling it differently.
Fair point. In which case, `SHT_LLVM_BITCODE` seems more self-descriptive (describing what it is rather than one potential use for it)? I imagine you could do things other than LTO with it in the future too after all :)

I'm not necessarily opposed to `SHT_LLVM_LTO`, but as this name is going to live a very long time, it's important we get it right!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153215