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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

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

> In D128382#3603513 <https://reviews.llvm.org/D128382#3603513>, @MaskRay wrote:
>
>> Please answer why `executableInputSections > 1` is needed.
>
> If there is zero or only one input section having instructions, the scenario of "hot code calling into cold code" doesn't exist.

Then just let existing code handle it?

> In D128382#3603513 <https://reviews.llvm.org/D128382#3603513>, @MaskRay wrote:
>
>> How does it save one page?
>
> Hot code will always be placed together, so where it starts impact how many pages it will occupy.  Moving it towards the beginning of the output section increases the possibility that one less page will be taken.

I am not sure this is true.
For -z separate-code layout, PT_LOAD program header has an aligned start address. I agree that placing hot code at the start may potentially remove one hot page.
For -z noseparate-code layout, I think we can construct a case that placing hot code at the start may use one more page.

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

> In D128382#3603513 <https://reviews.llvm.org/D128382#3603513>, @MaskRay wrote:
>
>> 4 size=0xC00000 input sections are too large (48MiB). If just for `ARM::getThunkSectionSpacing` (16MiB), it is too much.
>> Please check split-file. I think we need tests when ` totalSize >= target->getThunkSectionSpacing()` triggers or not.
>
> The original test covers the scenario where `totalSize < target->getThunkSectionSpacing()`.  The new test covers the opposite.

OK. Please use `split-file`. Placing the two cases in one file will be clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128382