[PATCH] D56325: Sort symbols in .bss by size.

Vic Yang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 13 21:14:45 PST 2019


victoryang added a comment.

In D56325#1355019 <https://reviews.llvm.org/D56325#1355019>, @ruiu wrote:

> In D56325#1353178 <https://reviews.llvm.org/D56325#1353178>, @grimar wrote:
>
> > In D56325#1353138 <https://reviews.llvm.org/D56325#1353138>, @ruiu wrote:
> >
> > > I don't think we need a detailed benchmark for other targets, as how programs use .bss (and in general other parts of data sections) doesn't depend too much on targets.
> >
> >
> > Hard to say. I remember we had a patch that fixed inconsistency of handling .bss symbols (we had an issue in a case when application wanted to have the same order of symbols it would have if we would create them in a command line order, it had a relocation overflow because we create the .bss symbols first now). The patch was declined (I can find it if you want). So if this patch solves a local issue of low-end Android handsets only I really see no reason of why we want to change the behavior of LLD? It is the almost the same case that patch had. The difference is that now it is needed for Android and not for an unknown application. Until we have benchmarks showing it has a benefit for other platforms I am not convinced we should change the linker. It is up to you anyways, I am just saying my opinion about that change.
>
>
> For the record, the reason why I'm in favor of this patch not because I'm working for Google; it is not correct to say that this patch is to fix a local issue. I've been trying to handle all contributors equally. As to this patch, I think it doesn't only improve Android but improve every target. Android Go team is working hard to reduce memory usage, and that's why they found this heuristics, and I added that to the comment because a concrete example is more reader-friendly than saying the same thing abstractly. But nothing special about Android Go.
>
> That said, I can ask Vic to build other regular Linux applications with/without this patch to see if the same improvement (or at least no regression) can be observed. Vic, do you mind if I ask you to do that?


I don't mind doing the test. I spent some time trying to find a open source project for this test, but I'm having a hard time finding one that 1) uses clang, 2) has a large enough bss section for this test, and 3) doesn't take me crazy amount of time to setup the build. Before I spend more time on this, I thought I'd ask if you have any suggestion on what programs to test this on.


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

https://reviews.llvm.org/D56325





More information about the llvm-commits mailing list