[llvm] r322595 - Specify inline for isWhitespace in CommandLine.cpp

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 22 13:45:13 PST 2018


Yeah, this really shouldn't be necessary and seems really questionable... I
cannot imagine a way that LLVM requires this, and if it LLVM doesn't
require this I wonder if we should just move to benchmarking a self-hosted
LLD rather than worrying about the very detailed performance
characteristics with MSCV-built LLD. This seems like a pretty glaring
deficiency in the MSVC inliner that we're papering over.

On Mon, Jan 22, 2018 at 9:57 AM David Blaikie <dblaikie at gmail.com> wrote:

> Is this with a selfhost build? (ie: is clang failing to inline this
> function where it should) - should we look into why the compiler is failing
> to catch this case?
>
>
> On Tue, Jan 16, 2018 at 12:53 PM Rui Ueyama via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: ruiu
>> Date: Tue Jan 16 12:52:32 2018
>> New Revision: 322595
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=322595&view=rev
>> Log:
>> Specify inline for isWhitespace in CommandLine.cpp
>>
>> Patch by Takuto Ikuta.
>>
>> In chromium's component build, there are many directive sections and
>> commandline parsing takes much time.
>> This patch is for speed up of lld in RelWithDebInfo build by forcing
>> inline heavily called isWhitespace function.
>>
>> 10 times link perf stats of blink_core.dll changed like below.
>>
>> master:
>> TotalSeconds: 9.8764878
>> TotalSeconds: 10.1455242
>> TotalSeconds: 10.075279
>> TotalSeconds: 10.3397347
>> TotalSeconds: 9.8361665
>> TotalSeconds: 9.9544441
>> TotalSeconds: 9.8960686
>> TotalSeconds: 9.8877865
>> TotalSeconds: 10.0551879
>> TotalSeconds: 10.0492254
>> Avg: 10.01159047
>>
>> with this patch:
>> TotalSeconds: 8.8696762
>> TotalSeconds: 9.1021585
>> TotalSeconds: 9.0233893
>> TotalSeconds: 9.1886175
>> TotalSeconds: 9.156954
>> TotalSeconds: 9.0978564
>> TotalSeconds: 9.1316824
>> TotalSeconds: 8.8354606
>> TotalSeconds: 9.2549431
>> TotalSeconds: 9.4473085
>> Avg: 9.11080465
>>
>> Modified:
>>     llvm/trunk/lib/Support/CommandLine.cpp
>>
>> Modified: llvm/trunk/lib/Support/CommandLine.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=322595&r1=322594&r2=322595&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Support/CommandLine.cpp (original)
>> +++ llvm/trunk/lib/Support/CommandLine.cpp Tue Jan 16 12:52:32 2018
>> @@ -688,7 +688,7 @@ static bool EatsUnboundedNumberOfValues(
>>           O->getNumOccurrencesFlag() == cl::OneOrMore;
>>  }
>>
>> -static bool isWhitespace(char C) {
>> +static inline bool isWhitespace(char C) {
>>    return C == ' ' || C == '\t' || C == '\r' || C == '\n';
>>  }
>>
>>
>>
>> _______________________________________________
>> 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/20180122/0c9ed5f5/attachment.html>


More information about the llvm-commits mailing list