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

Mon Oct 26 14:48:32 PDT 2015

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)

> +              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)

> +
>    /// 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/35fcbe7c/attachment.html>