[llvm] r292420 - [ThinLTO] Add a recursive step in Metadata lazy-loading

[Hans Wennborg via llvm-commits]

Sounds good to me. Mehdi, go ahead and merge or let me know if you'd
like me to do it.

Thanks,
Hans

On Wed, Jan 18, 2017 at 12:19 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
> Approved.
>
> Peter
>
> On Wed, Jan 18, 2017 at 10:51 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>>
>> Hi Peter,
>>
>> I’d like to cherry-pick this into the 4.0 release. This is a follow-up to
>> r291027, it achieve what was originally promised.
>>
>> It will only affect ThinLTO, as lazy-loading happens only there.
>>
>> Thanks,
>>
>> —
>> Mehdi
>>
>> > On Jan 18, 2017, at 10:36 AM, Mehdi Amini via llvm-commits
>> > <llvm-commits at lists.llvm.org> wrote:
>> >
>> > Author: mehdi_amini
>> > Date: Wed Jan 18 12:36:21 2017
>> > New Revision: 292420
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=292420&view=rev
>> > Log:
>> > [ThinLTO] Add a recursive step in Metadata lazy-loading
>> >
>> > Summary:
>> > Without this, we're stressing the RAUW of unique nodes,
>> > which is a costly operation. This is intended to limit
>> > the number of RAUW, and is very effective on the total
>> > link-time of opt with ThinLTO, before:
>> >
>> >  real 4m4.587s  user 15m3.401s  sys 0m23.616s
>> >
>> > after:
>> >
>> >  real 3m25.261s user 12m22.132s sys 0m24.152s
>> >
>> > Reviewers: tejohnson, pcc
>> >
>> > Subscribers: llvm-commits
>> >
>> > Differential Revision: https://reviews.llvm.org/D28751
>> >
>> > Modified:
>> >    llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp
>> >    llvm/trunk/test/ThinLTO/X86/lazyload_metadata.ll
>> >
>> > Modified: llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp?rev=292420&r1=292419&r2=292420&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp (original)
>> > +++ llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp Wed Jan 18 12:36:21
>> > 2017
>> > @@ -768,13 +768,12 @@ void MetadataLoader::MetadataLoaderImpl:
>> >     unsigned ID, PlaceholderQueue &Placeholders) {
>> >   assert(ID < (MDStringRef.size()) + GlobalMetadataBitPosIndex.size());
>> >   assert(ID >= MDStringRef.size() && "Unexpected lazy-loading of
>> > MDString");
>> > -#ifndef NDEBUG
>> >   // Lookup first if the metadata hasn't already been loaded.
>> >   if (auto *MD = MetadataList.lookup(ID)) {
>> >     auto *N = dyn_cast_or_null<MDNode>(MD);
>> > -    assert(N && N->isTemporary() && "Lazy loading an already loaded
>> > metadata");
>> > +    if (!N->isTemporary())
>> > +      return;
>> >   }
>> > -#endif
>> >   SmallVector<uint64_t, 64> Record;
>> >   StringRef Blob;
>> >   IndexCursor.JumpToBit(GlobalMetadataBitPosIndex[ID -
>> > MDStringRef.size()]);
>> > @@ -827,8 +826,22 @@ Error MetadataLoader::MetadataLoaderImpl
>> >   auto getMD = [&](unsigned ID) -> Metadata * {
>> >     if (ID < MDStringRef.size())
>> >       return lazyLoadOneMDString(ID);
>> > -    if (!IsDistinct)
>> > +    if (!IsDistinct) {
>> > +      if (auto *MD = MetadataList.lookup(ID))
>> > +        return MD;
>> > +      // If lazy-loading is enabled, we try recursively to load the
>> > operand
>> > +      // instead of creating a temporary.
>> > +      if (ID < (MDStringRef.size() + GlobalMetadataBitPosIndex.size()))
>> > {
>> > +        // Create a temporary for the node that is referencing the
>> > operand we
>> > +        // will lazy-load. It is needed before recursing in case there
>> > are
>> > +        // uniquing cycles.
>> > +        MetadataList.getMetadataFwdRef(NextMetadataNo);
>> > +        lazyLoadOneMetadata(ID, Placeholders);
>> > +        return MetadataList.lookup(ID);
>> > +      }
>> > +      // Return a temporary.
>> >       return MetadataList.getMetadataFwdRef(ID);
>> > +    }
>> >     if (auto *MD = MetadataList.getMetadataIfResolved(ID))
>> >       return MD;
>> >     return &Placeholders.getPlaceholderOp(ID);
>> >
>> > Modified: llvm/trunk/test/ThinLTO/X86/lazyload_metadata.ll
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/lazyload_metadata.ll?rev=292420&r1=292419&r2=292420&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/test/ThinLTO/X86/lazyload_metadata.ll (original)
>> > +++ llvm/trunk/test/ThinLTO/X86/lazyload_metadata.ll Wed Jan 18 12:36:21
>> > 2017
>> > @@ -17,7 +17,7 @@
>> > ; RUN:          -o /dev/null -disable-ondemand-mds-loading -stats \
>> > ; RUN:  2>&1 | FileCheck %s -check-prefix=NOTLAZY
>> > ; NOTLAZY: 58 bitcode-reader  - Number of Metadata records loaded
>> > -; NOTLAZY: 8 bitcode-reader  - Number of MDStrings loaded
>> > +; NOTLAZY: 6 bitcode-reader  - Number of MDStrings loaded
>> >
>> >
>> > target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
>> > @@ -48,7 +48,7 @@ define void @globalfunc3(i32 %arg) {
>> > !3 = !{!"3"}
>> > !4 = !{!"4"}
>> > !5 = !{!"5"}
>> > -!6 = !{!"6"}
>> > +!6 = !{!9}
>> > !7 = !{!"7"}
>> > !8 = !{!"8"}
>> > -!9 = !{!"9"}
>> > +!9 = !{!6}
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
>
> --
> --
> Peter