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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a subscriber: srhines.
MaskRay added a comment.

This patch tries to adjust D44969 <https://reviews.llvm.org/D44969> heuristics a bit.

> This is not necessary if less than two input sections contain anything that is executable, for example, when user specifies an ordering of read-only data (instead of function) symbols. It is also not necessary if total size of output section is small such

that no branch thunk would ever be required, which is common for mobile apps.

I agree that the mentioned cases do not unnecessarily need reordering. It doesn't matter either way, then why change the behavior with more code?

In D128382#3603081 <https://reviews.llvm.org/D128382#3603081>, @smeenai wrote:

> You should also add the motivation for this change to the description (which I believe is to minimize the number of pages occupied by hot code).

Such a motivation is fine. Perhaps @srhines can test this for other applications.



================
Comment at: lld/ELF/Writer.cpp:1325
+    if (isec->flags & SHF_EXECINSTR)
+      executableInputSections++;
     auto i = order.find(isec);
----------------
https://llvm.org/docs/CodingStandards.html#prefer-preincrement


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128382