[llvm] r251353 - Move imported entities into DwarfCompilationUnit to speed up LTO linking.

Mon Oct 26 14:54:35 PDT 2015

On Mon, Oct 26, 2015 at 2:48 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Mon, Oct 26, 2015 at 2:36 PM, Ivan Krasin via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: krasin
>> Date: Mon Oct 26 16:36:35 2015
>> New Revision: 251353
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=251353&view=rev
>> Log:
>> Move imported entities into DwarfCompilationUnit to speed up LTO linking.
>>
>> Summary:
>> In particular, this CL speeds up the official Chrome linking with LTO by
>> 1.8x.
>>
>> See more details in https://crbug.com/542426
>>
>> Reviewers: dblaikie
>>
>> Subscribers: jevinskie
>>
>> Differential Revision: http://reviews.llvm.org/D13918
>>
>> Modified:
>>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
>>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=251353&r1=251352&r2=251353&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp Mon Oct 26
>> 16:36:35 2015
>> @@ -342,9 +342,9 @@ void DwarfCompileUnit::constructScopeDIE
>>      // Skip imported directives in gmlt-like data.
>>      if (!includeMinimalInlineScopes()) {
>>        // There is no need to emit empty lexical block DIE.
>> -      for (const auto &E : DD->findImportedEntitiesForScope(DS))
>> -        Children.push_back(
>> -
>> constructImportedEntityDIE(cast<DIImportedEntity>(E.second)));
>> +      for (const auto *IE : ImportedEntities[DS])
>> +          Children.push_back(
>>
>
> Looks like an extra indent happened on the line above (seems like the body
> of the for loop is indented 4 spaces instead of 2)
>
Looking...

>
>
>> +              constructImportedEntityDIE(cast<DIImportedEntity>(IE)));
>>      }
>>
>>      // If there are only other scopes as children, put them directly in
>> the
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h?rev=251353&r1=251352&r2=251353&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h Mon Oct 26
>> 16:36:35 2015
>> @@ -39,6 +39,12 @@ class DwarfCompileUnit : public DwarfUni
>>    /// The start of the unit within its section.
>>    MCSymbol *LabelBegin;
>>
>> +  typedef llvm::SmallVector<const MDNode *, 8> ImportedEntityList;
>> +  typedef llvm::DenseMap<const MDNode *, ImportedEntityList>
>> +  ImportedEntityMap;
>> +
>> +  ImportedEntityMap ImportedEntities;
>>
>
> Are you going to experiment with switching this back to a sorted sequence?
> Might be a nice memory savings over the overhead of densemap+vectors?
> Possibly a runtime savings too in terms of building the structures
> (building, sorting, then searching can be cheaper than incremental building
> maintaining the sorted invariant) Or was this actually better performing
> than the original data structure choice? (even once moved to a per-CU level)
>
I tried the sorted sequence. It was either slower, or ugly. Depends on the
variant I tried. As for memory consumption, we'll see if this change make
the situation much worse.
For the reference, linking Chrome with LTO (this patch included) requires
~32 GB of RAM.

>
>
>> +
>>    /// GlobalNames - A map of globally visible named entities for this
>> unit.
>>    StringMap<const DIE *> GlobalNames;
>>
>> @@ -98,6 +104,10 @@ public:
>>
>>    unsigned getOrCreateSourceID(StringRef FileName, StringRef DirName)
>> override;
>>
>> +  void addImportedEntity(const DIImportedEntity* IE) {
>> +    ImportedEntities[IE->getScope()].push_back(IE);
>> +  }
>> +
>>    /// addRange - Add an address range to the list of ranges for this
>> unit.
>>    void addRange(RangeSpan Range);
>>
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=251353&r1=251352&r2=251353&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Oct 26 16:36:35
>> 2015
>> @@ -489,12 +489,7 @@ void DwarfDebug::beginModule() {
>>      auto *CUNode = cast<DICompileUnit>(N);
>>      DwarfCompileUnit &CU = constructDwarfCompileUnit(CUNode);
>>      for (auto *IE : CUNode->getImportedEntities())
>> -
>> ScopesWithImportedEntities.push_back(std::make_pair(IE->getScope(), IE));
>> -    // Stable sort to preserve the order of appearance of imported
>> entities.
>> -    // This is to avoid out-of-order processing of interdependent
>> declarations
>> -    // within the same scope, e.g. { namespace A = base; namespace B =
>> A; }
>> -    std::stable_sort(ScopesWithImportedEntities.begin(),
>> -                     ScopesWithImportedEntities.end(), less_first());
>> +      CU.addImportedEntity(IE);
>>      for (auto *GV : CUNode->getGlobalVariables())
>>        CU.getOrCreateGlobalVariableDIE(GV);
>>      for (auto *SP : CUNode->getSubprograms())
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=251353&r1=251352&r2=251353&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Mon Oct 26 16:36:35
>> 2015
>> @@ -289,11 +289,6 @@ class DwarfDebug : public AsmPrinterHand
>>    /// Holders for the various debug information flags that we might need
>> to
>>    /// have exposed. See accessor functions below for description.
>>
>> -  /// Holder for imported entities.
>> -  typedef SmallVector<std::pair<const MDNode *, const MDNode *>, 32>
>> -  ImportedEntityMap;
>> -  ImportedEntityMap ScopesWithImportedEntities;
>> -
>>    /// Map from MDNodes for user-defined types to the type units that
>>    /// describe them.
>>    DenseMap<const MDNode *, const DwarfTypeUnit *> DwarfTypeUnits;
>> @@ -626,14 +621,6 @@ public:
>>
>>    const MachineFunction *getCurrentFunction() const { return CurFn; }
>>
>> -  iterator_range<ImportedEntityMap::const_iterator>
>> -  findImportedEntitiesForScope(const MDNode *Scope) const {
>> -    return make_range(std::equal_range(
>> -        ScopesWithImportedEntities.begin(),
>> ScopesWithImportedEntities.end(),
>> -        std::pair<const MDNode *, const MDNode *>(Scope, nullptr),
>> -        less_first()));
>> -  }
>> -
>>    /// A helper function to check whether the DIE for a given Scope is
>>    /// going to be null.
>>    bool isLexicalScopeDIENull(LexicalScope *Scope);
>>
>>
>> _______________________________________________
>> 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/20151026/fe029597/attachment.html>