<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 26, 2015 at 2:54 PM, Ivan Krasin <span dir="ltr"><<a href="mailto:krasin@google.com" target="_blank">krasin@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Mon, Oct 26, 2015 at 2:48 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 26, 2015 at 2:36 PM, Ivan Krasin via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: krasin<br>
Date: Mon Oct 26 16:36:35 2015<br>
New Revision: 251353<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=251353&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=251353&view=rev</a><br>
Log:<br>
Move imported entities into DwarfCompilationUnit to speed up LTO linking.<br>
<br>
Summary:<br>
In particular, this CL speeds up the official Chrome linking with LTO by<br>
1.8x.<br>
<br>
See more details in <a href="https://crbug.com/542426" rel="noreferrer" target="_blank">https://crbug.com/542426</a><br>
<br>
Reviewers: dblaikie<br>
<br>
Subscribers: jevinskie<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D13918" rel="noreferrer" target="_blank">http://reviews.llvm.org/D13918</a><br>
<br>
Modified:<br>
llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp<br>
llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h<br>
llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=251353&r1=251352&r2=251353&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=251353&r1=251352&r2=251353&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp Mon Oct 26 16:36:35 2015<br>
@@ -342,9 +342,9 @@ void DwarfCompileUnit::constructScopeDIE<br>
// Skip imported directives in gmlt-like data.<br>
if (!includeMinimalInlineScopes()) {<br>
// There is no need to emit empty lexical block DIE.<br>
- for (const auto &E : DD->findImportedEntitiesForScope(DS))<br>
- Children.push_back(<br>
- constructImportedEntityDIE(cast<DIImportedEntity>(E.second)));<br>
+ for (const auto *IE : ImportedEntities[DS])<br>
+ Children.push_back(<br></blockquote><div><br></div><div>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)</div></div></div></div></blockquote></div></div><div>Looking... </div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ constructImportedEntityDIE(cast<DIImportedEntity>(IE)));<br>
}<br>
<br>
// If there are only other scopes as children, put them directly in the<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h?rev=251353&r1=251352&r2=251353&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h?rev=251353&r1=251352&r2=251353&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h Mon Oct 26 16:36:35 2015<br>
@@ -39,6 +39,12 @@ class DwarfCompileUnit : public DwarfUni<br>
/// The start of the unit within its section.<br>
MCSymbol *LabelBegin;<br>
<br>
+ typedef llvm::SmallVector<const MDNode *, 8> ImportedEntityList;<br>
+ typedef llvm::DenseMap<const MDNode *, ImportedEntityList><br>
+ ImportedEntityMap;<br>
+<br>
+ ImportedEntityMap ImportedEntities;<br></blockquote><div><br></div><div>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)</div></div></div></div></blockquote></span><div>I tried the sorted sequence. It was either slower, or ugly. </div></div></div></div></blockquote><div><br></div><div>Curious, but fair enough. (yeah, I suppose the ugly part would be needing an explicit hook into DwarfCompileUnit to say "I've finished adding all the entities, sort them now" (but make sure you don't use them until then, because the sorted invariant won't be available))<br><br>What if we built the collection directly in the population code, sorted it, then just had "DwarfCompileUnit::setImportedEntities" (& pass the collection by value/move, which should be cheap/free)? Or is that the sort of thing you found to be ugly?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Depends on the variant I tried. As for memory consumption, we'll see if this change make the situation much worse.</div><div>For the reference, linking Chrome with LTO (this patch included) requires ~32 GB of RAM.</div></div></div></div></blockquote><div><br></div><div>Fair enough</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
/// GlobalNames - A map of globally visible named entities for this unit.<br>
StringMap<const DIE *> GlobalNames;<br>
<br>
@@ -98,6 +104,10 @@ public:<br>
<br>
unsigned getOrCreateSourceID(StringRef FileName, StringRef DirName) override;<br>
<br>
+ void addImportedEntity(const DIImportedEntity* IE) {<br>
+ ImportedEntities[IE->getScope()].push_back(IE);<br>
+ }<br>
+<br>
/// addRange - Add an address range to the list of ranges for this unit.<br>
void addRange(RangeSpan Range);<br>
<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=251353&r1=251352&r2=251353&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=251353&r1=251352&r2=251353&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Oct 26 16:36:35 2015<br>
@@ -489,12 +489,7 @@ void DwarfDebug::beginModule() {<br>
auto *CUNode = cast<DICompileUnit>(N);<br>
DwarfCompileUnit &CU = constructDwarfCompileUnit(CUNode);<br>
for (auto *IE : CUNode->getImportedEntities())<br>
- ScopesWithImportedEntities.push_back(std::make_pair(IE->getScope(), IE));<br>
- // Stable sort to preserve the order of appearance of imported entities.<br>
- // This is to avoid out-of-order processing of interdependent declarations<br>
- // within the same scope, e.g. { namespace A = base; namespace B = A; }<br>
- std::stable_sort(ScopesWithImportedEntities.begin(),<br>
- ScopesWithImportedEntities.end(), less_first());<br>
+ CU.addImportedEntity(IE);<br>
for (auto *GV : CUNode->getGlobalVariables())<br>
CU.getOrCreateGlobalVariableDIE(GV);<br>
for (auto *SP : CUNode->getSubprograms())<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=251353&r1=251352&r2=251353&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=251353&r1=251352&r2=251353&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Mon Oct 26 16:36:35 2015<br>
@@ -289,11 +289,6 @@ class DwarfDebug : public AsmPrinterHand<br>
/// Holders for the various debug information flags that we might need to<br>
/// have exposed. See accessor functions below for description.<br>
<br>
- /// Holder for imported entities.<br>
- typedef SmallVector<std::pair<const MDNode *, const MDNode *>, 32><br>
- ImportedEntityMap;<br>
- ImportedEntityMap ScopesWithImportedEntities;<br>
-<br>
/// Map from MDNodes for user-defined types to the type units that<br>
/// describe them.<br>
DenseMap<const MDNode *, const DwarfTypeUnit *> DwarfTypeUnits;<br>
@@ -626,14 +621,6 @@ public:<br>
<br>
const MachineFunction *getCurrentFunction() const { return CurFn; }<br>
<br>
- iterator_range<ImportedEntityMap::const_iterator><br>
- findImportedEntitiesForScope(const MDNode *Scope) const {<br>
- return make_range(std::equal_range(<br>
- ScopesWithImportedEntities.begin(), ScopesWithImportedEntities.end(),<br>
- std::pair<const MDNode *, const MDNode *>(Scope, nullptr),<br>
- less_first()));<br>
- }<br>
-<br>
/// A helper function to check whether the DIE for a given Scope is<br>
/// going to be null.<br>
bool isLexicalScopeDIENull(LexicalScope *Scope);<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>