[PATCH] D27147: Put always_inline on adjustExpr

[Sean Silva via llvm-commits]

On Mon, Nov 28, 2016 at 9:37 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Sat, Nov 26, 2016 at 11:31 PM Sean Silva via Phabricator via
> llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
>> silvas created this revision.
>> silvas added reviewers: ruiu, rafael.
>> silvas added a subscriber: llvm-commits.
>> silvas set the repository for this revision to rL LLVM.
>>
>> This function is incredibly hot (it is called unconditionally for every
>> relocation) and so we need everything the compiler can give us for it. It
>> is just a helper anyway.
>>
>> This change is good for 3-4% speedup on clang-fsds with `ld.lld -O1`.
>>
>> The compiler that needed this identifies itself as `gcc (Ubuntu
>> 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609`
>>
>
> Is it important to optmize our code for this compiler? My general
> understanding was that we mostly focus on selfhost perf for LLVM projects -
> correctness everywhere, then just bootstrap to get good perf. That way we
> can just fix compiler bugs in the compiler rather than both in the compiler
> and in the project source.
>

This is a fine idea in theory, but in practice we don't want LLD too
sensitive to choice of compiler. We don't want to have to qualify LLD's
performance claims with a huge list of fine print about how you need to
build it. For example, a Debian or Gentoo packager or the build of LLD for
the FreeBSD base system is unlikely to be willing to do a PGO build, nor
will they necessarily be willing to compile LLD with the compiler (or
compiler version) of our choice. Our users should not suffer for this if we
can avoid it pretty easily.

LLD is very different from Clang or LLVM -- both Clang and LLVM are very
large (at least an order of magnitude or two larger than LLD could ever
become) and have a pretty flat profile: there is a real software
engineering maintainability question about whether it is worth it to
annotate for the compiler, because you would have to annotate so many
places in order to get any significant gain. PGO is a good fit for that,
since it avoids the software engineering issues by letting the compiler
iterate over the entire program and get lots of tiny gains. However, for
LLD it is different; LLD is a much smaller codebase and spends almost all
of its time in just a handful of functions (this is slight hyperbole, but
the profile is very concentrated and not flat at all). So a very small and
manageable set of targeted fixes can help a lot.

E.g. Rui's https://reviews.llvm.org/D27155 improves a key performance
metric (building a single debug binary on an otherwise unloaded machine) by
25% by swapping out a single isolated subroutine with a faster
implementation.

-- Sean Silva


>
>
>> Also, this will allow us in the future to more accurately assess the
>> benefits of PGO for building LLD. If we can get this speedup easily without
>> PGO by just applying an attribute, we don't want this 3-4% to inflate the
>> benefit of PGO when we try PGO.
>>
>
> I'm not sure that's necessarily an inflation (though if LLVM is missing
> this inline opportunity that it should be getting even without PGO, etc -
> then we should fix LLVM) - I think PGO should help LLVM get situations like
> this that a user /could/ fix, but finding and fixing them all is
> expensive/not practical overall, so having a tool that can get them for you
> is valuable.
>
> - Dave
>
>
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D27147
>>
>> Files:
>>   ELF/Relocations.cpp
>>
>>
>> Index: ELF/Relocations.cpp
>> ===================================================================
>> --- ELF/Relocations.cpp
>> +++ ELF/Relocations.cpp
>> @@ -421,10 +421,10 @@
>>  }
>>
>>  template <class ELFT>
>> -static RelExpr adjustExpr(const elf::ObjectFile<ELFT> &File, SymbolBody
>> &Body,
>> -                          bool IsWrite, RelExpr Expr, uint32_t Type,
>> -                          const uint8_t *Data, InputSectionBase<ELFT> &S,
>> -                          typename ELFT::uint RelOff) {
>> +LLVM_ATTRIBUTE_ALWAYS_INLINE static inline RelExpr
>> +adjustExpr(const elf::ObjectFile<ELFT> &File, SymbolBody &Body, bool
>> IsWrite,
>> +           RelExpr Expr, uint32_t Type, const uint8_t *Data,
>> +           InputSectionBase<ELFT> &S, typename ELFT::uint RelOff) {
>>    bool Preemptible = isPreemptible(Body, Type);
>>    if (Body.isGnuIFunc()) {
>>      Expr = toPlt(Expr);
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161130/740e163a/attachment.html>