[PATCH] D65242: [ELF] More dynamic relocation packing

[Vic Yang via Phabricator via llvm-commits]

victoryang added a comment.

In D65242#1605239 <https://reviews.llvm.org/D65242#1605239>, @pcc wrote:

> In D65242#1605129 <https://reviews.llvm.org/D65242#1605129>, @victoryang wrote:
>
> > In D65242#1604903 <https://reviews.llvm.org/D65242#1604903>, @pcc wrote:
> >
> > > In D65242#1604869 <https://reviews.llvm.org/D65242#1604869>, @victoryang wrote:
> > >
> > > > In D65242#1604825 <https://reviews.llvm.org/D65242#1604825>, @pcc wrote:
> > > >
> > > > > > Linking libmonochrome in Chromium for
> > > > > >  Android, targeting ARM32, I see a .rel.dyn of size 127697 bytes, as
> > > > > >  compared to 150532 bytes without this change.
> > > > >
> > > > > IIRC over 99% of relocations in libmonochrome were relative, so I didn't implement anything for the non-relative relocations, but maybe there are more non-relative relocations now. I can imagine that if say you were building it as a component build that would result in more non-relative relocations than in shipping builds. It may also be more compelling to mention how much this helps for the Android platform which I believe uses non-relative relocations more heavily.
> > > >
> > > >
> > > > Thanks for pointing this out! I've updated the description to include test results on Android.
> > >
> > >
> > > Thanks for adding those figures. I'm still not sure about your Chromium figures though. I just did a quick build myself and I'm still seeing >99% relative relocations without significant duplication in the non-relative relocations so I'm curious to see what your `args.gn` looks like for Chromium.
> >
> >
> > My args.gn is:
> >
> >   target_os = "android"
> >   use_goma = true
> >
> >
> > Also FWIW I'm building a debug build.
>
>
> Okay, since that's a component build and not realistic as a size measurement for anything that's shipped I probably wouldn't mention the Chromium numbers in the commit message since they're somewhat misleading.


Fair enough. I removed the part about Chromium. Just for my own education, what's the right way of testing this for Chromium? Is it just that I should do a Release build than a Debug build?

> 
> 
>>>>> It may also be worth compressing `.rela.plt` now that we're compressing non-relative relocations.
>>>> 
>>>> Do we expect duplicated symbol lookup in .rela.plt? Honestly I'm not sure, but I'm under the impression that we'd have one PLT entry for each external function, so we won't see duplication?
>>> 
>>> You may have duplication between `.rela.plt` and `.rela.dyn` if a function is both called and address taken. But if there are many PLT entries then you should expect to see an improvement just from compressing the relocations themselves since right now they're being stored uncompressed.
>> 
>> Compressing across sections doesn't seem like something we can do right now, but I'll look into other compression you mentioned.
> 
> We could just emit the relocations that would otherwise go to `.rela.plt` into `.rela.dyn` in the case where `-z now` is passed. Then they'll get compressed automatically. But that's largely orthogonal to what you're doing here so it can be done as a separate patch.




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

https://reviews.llvm.org/D65242