[PATCH] D26130: [ELF] - Implemented --symbol-ordering-file option.

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 23 12:12:30 PST 2016


On Mon, Nov 21, 2016 at 3:10 PM, Rui Ueyama via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> I don't think it's proved to work. It might need improving until we provide
> it to the public.
>

I'm afraid I have to disagree with you, at least partly. As you can
see I was the first one being very skeptical of the benchmarks run
(clang --help), but it turns out that Rafael did an analysis himself
and showed some benefit of the feature.
With that in mind, I agree there might be cases we didn't consider
properly and the feature itself requires more testing.

> In particular, I'm worried that this feature could produce unexpected
> section ordering, because it reorders local symbols. Local symbols are not
> guaranteed to be unique, so a symbol in a symbol ordering file could
> accidentally matches with an unintended symbol and reorders sections
> wrongly. gold's --section-ordering-file might be half-baked, but this
> --symbol-orderfing-file might be half-baked, too.
>
> More importantly, I think my point is that we should have had this kind of
> discussion based on a proposal before landing a feature.
>

I don't disagree. I was under the impression that this discussion
happened as part of the review, but if you have concerns, it clearly
didn't. I pointed out that we plan to use this feature internally, and
I tried to express my concerns on the review (although I didn't author
the code).
My take is that, maybe, at this point, it's easier to ask the author
(i.e. George) to properly run benchmarks also in pathological cases
and test more applications.
To me, the class of application most sensitive to this kind of
performance optimization are the ones with long startup times (e.g.
browser), but if you have something else in mind that can be tested,
feel free to share.
4.0 is still a little bit (far?) away, so I'd rather iterate this
in-tree and back it out before the release if we don't think it's
ready.

Thanks,

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare


More information about the llvm-commits mailing list