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

Thu Jun 23 03:26:25 PDT 2022

srhines added a subscriber: trybka.
srhines added a comment.

In D128382#3603142 <https://reviews.llvm.org/D128382#3603142>, @MaskRay wrote:

> 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.

This optimization requires profile data, which isn't generally in use for 1p NDK usage (to my knowledge). Maybe @trybka might know if there are PGO/AFDO users in our 1p apps. As for the Android platform, we do use AutoFDO, but it isn't possible for us to collect enough AFDO data to validate this a priori. We'd have to merge this patch into a toolchain that is currently being used for droidfood, and then experiment later to see what impact it had. Based on the way that Android schedules work, it would be months before we have enough droidfooders on next year's release. I think it might be best to see if this has an impact on any Google 1p apps for now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128382