[PATCH] D128382: [LLD] Two tweaks to symbol ordering scheme

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D128382#3611193 <https://reviews.llvm.org/D128382#3611193>, @yozhu wrote:

> In D128382#3610276 <https://reviews.llvm.org/D128382#3610276>, @MaskRay wrote:
>
>> In D128382#3609648 <https://reviews.llvm.org/D128382#3609648>, @lanza wrote:
>>
>>> One interesting note Shoaib and I just discovered is that ~16.5% of the libraries in one of our major apps have `.text` sections starting on the first page of the dso. Placing hot sections at the front of the `.text` section would give us a slice of a "free page" that, with the current algorithm, would just be cold code that's less likely to be used. I imagine this isn't a substantial difference but it's probably slightly in favor of placing hot code first here.
>>
>> Thanks. This is fine, even if not that large. The patch should use `osec->flags & SHF_EXECINSTR` instead of `executableInputSections` for simplification. Then the complexity/benefit trade-off will look fine.
>
> I didn't check output section's attribute but instead check on each input section because:
>
> - It could be that the output section contains .text (RX) contributions as well as .rdata (R) contributions, like read-only data is merged into text section.

This implies a linker script placing rodata into an executable output section.
This is a corner case and the operation here does not matter. Just be simple.

> - It could be that output section is not marked as executable, but it does contain instructions and at runtime the output section might be reprotected to be executable.

No. The output section generally has all flags from input sections. See `OutputSection::commitSection`

> So here we "trust" more on what compiler/assembler says on each input section's attribute.  And since we are enumerating through all input sections anyway, counting `executableInputSections` doesn't add too much complication.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128382