[PATCH] D150510: [ELF] x86-64: place .lrodata, .lbss, and .ldata away from code sections

Wed May 24 13:27:18 PDT 2023

MaskRay marked an inline comment as done.
MaskRay added a comment.

In D150510#4368945 <https://reviews.llvm.org/D150510#4368945>, @aeubanks wrote:

> In D150510#4364809 <https://reviews.llvm.org/D150510#4364809>, @jyknight wrote:
>
>> As a reminder, the section layout we're talking about is: `.data` (PROGBITS RW), `.bss` (NOBITS RW), `.ldata` (PROGBITS RW LARGE). The question is whether to cover all three sections with a single LOAD, or whether to use two LOADs, one for `.data` and `.bss`, and the second for `.ldata`.
>>
>> In the former case, you cannot use the ability of an ELF LOAD to specify a smaller filesize than memory memory-size -- where the remainder of the memory-size gets zero-filled by the loader, because that can only be at the end of the LOAD. Thus, in that model, if you have 8MB of `.bss`, you will waste 8MB of zeros in the binary.
>>
>> My patch does the latter, instead, and that saves space, already, without additional changes to other functions. Because of the way the alignment code works, you'll still waste some bytes in the file, but only up to max-page-size bytes (which defaults to 4K for x86-64; users can change it with `-z,max-page-size=` if they like).
>
> The size of `.bss` is likely non-negligible if we're implementing `-mlarge-data-threshold`, so this does seem important.
>
> In D150510#4362358 <https://reviews.llvm.org/D150510#4362358>, @MaskRay wrote:
>
>> If we start a new PT_LOAD at NOBITS->BITS boundaries, we can in theory implement such a file size optimization.
>> `fixSectionAlignments` and `assignFileOffsets` would need some involved code.
>> I think this is error-prone and may not be a worthwhile change.
>
> James is saying that this already works?
>
> anyway, this patch lgtm, I can take over James's https://github.com/llvm/llvm-project/commit/1ef68439208701146384d04f58286cdb94623452 for the new LOAD

Yes, it already works. Seems good to incorporate the change in this patch. We waste up to MAXPAGESIZE bytes, but the current behavior should be good enough.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150510