<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Aug 26, 2015 at 3:50 PM, Duncan P. N. Exon Smith 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: dexonsmith<br>
Date: Wed Aug 26 17:50:16 2015<br>
New Revision: 246098<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=246098&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=246098&view=rev</a><br>
Log:<br>
DI: Make Subprogram definitions 'distinct'<br>
<br>
Change `DIBuilder` always to produce 'distinct' nodes when creating<br>
`DISubprogram` definitions.  I measured a ~5% memory improvement in the<br>
link step (of ld64) when using `-flto -g`.<br>
<br>
`DISubprogram`s are used in two ways in the debug info graph.<br>
<br>
Some are definitions, point at actual functions, and can't really be<br>
shared between compile units.  With full debug info, these point down at<br>
their variables, forming uniquing cycles.  These uniquing cycles are<br>
expensive to link between modules, since all unique nodes that reference<br>
them transitively need to be duplicated (see commit message for r244181<br>
for more details).<br></blockquote><div><br>*reads r244181*<br><br>This does seem a bit awkward that we'll need to mark any regularly-cycle-creating nodes as distinct for performance reasons... :/ (but I imagine there's not much else to be done about it - ceratinly it's easy for the nodes that obviously wanted distinct-ness to begin with, but those that just happened across it due to a relationship like the subprogram->variable one (we only emit the variable list in optimized mode though, don't we?) seem quirk-ily unfortunate)<br><br>Speaking of the unoptimized case (that doesn't have a variable list, if I recall correctly - but maybe we changed that) - is this worse in any way to mark these nodes as distinct? Did they get naturally deduplicated previously & now have to be explicitly de duplicated? Not sure.<br><br>I guess at some point these will be attached to llvm::Functions directly, I think was the idea? (though not sure how we'll make a CU list of subprograms/functions to generate) Not sure if that'll change/simplify/make this less odd... </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Others are declarations, primarily used for member functions in the type<br>
hierarchy.  Definitions never show up there; instead, a definition<br>
points at its corresponding declaration node.<br>
<br>
I started by making all subprograms 'distinct'.  However, that was too<br>
big a hammer: memory usage *increased* ~5% (net increase vs. this patch<br>
of ~10%) because the 'distinct' declarations undermine LTO type<br>
uniquing.  This is a targeted fix for the definitions (where uniquing is<br>
an observable problem).<br>
<br>
A couple of notes:<br>
<br>
  - There's an accompanying commit to update IRGen testcases in clang.<br>
  - ^ That's what I'm using to test this commit.<br>
  - In a follow-up, I'll change the verifier to require 'distinct' on<br>
    definitions and add an upgrade to `BitcodeReader`.<br>
<br>
Modified:<br>
    llvm/trunk/lib/IR/DIBuilder.cpp<br>
<br>
Modified: llvm/trunk/lib/IR/DIBuilder.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=246098&r1=246097&r2=246098&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=246098&r1=246097&r2=246098&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/DIBuilder.cpp (original)<br>
+++ llvm/trunk/lib/IR/DIBuilder.cpp Wed Aug 26 17:50:16 2015<br>
@@ -675,6 +675,13 @@ DISubprogram *DIBuilder::createFunction(<br>
                         Flags, isOptimized, Fn, TParams, Decl);<br>
 }<br>
<br>
+template <class... Ts><br>
+static DISubprogram *getSubprogram(bool IsDistinct, Ts &&... Args) {<br>
+  if (IsDistinct)<br>
+    return DISubprogram::getDistinct(std::forward<Ts>(Args)...);<br>
+  return DISubprogram::get(std::forward<Ts>(Args)...);<br>
+}<br>
+<br>
 DISubprogram *DIBuilder::createFunction(DIScope *Context, StringRef Name,<br>
                                         StringRef LinkageName, DIFile *File,<br>
                                         unsigned LineNo, DISubroutineType *Ty,<br>
@@ -684,12 +691,13 @@ DISubprogram *DIBuilder::createFunction(<br>
                                         MDNode *TParams, MDNode *Decl) {<br>
   assert(Ty->getTag() == dwarf::DW_TAG_subroutine_type &&<br>
          "function types should be subroutines");<br>
-  auto *Node = DISubprogram::get(<br>
-      VMContext, DIScopeRef::get(getNonCompileUnitScope(Context)), Name,<br>
-      LinkageName, File, LineNo, Ty, isLocalToUnit, isDefinition, ScopeLine,<br>
-      nullptr, 0, 0, Flags, isOptimized, Fn, cast_or_null<MDTuple>(TParams),<br>
-      cast_or_null<DISubprogram>(Decl),<br>
-      MDTuple::getTemporary(VMContext, None).release());<br>
+  auto *Node = getSubprogram(/* IsDistinct = */ isDefinition, VMContext,<br>
+                             DIScopeRef::get(getNonCompileUnitScope(Context)),<br>
+                             Name, LinkageName, File, LineNo, Ty, isLocalToUnit,<br>
+                             isDefinition, ScopeLine, nullptr, 0, 0, Flags,<br>
+                             isOptimized, Fn, cast_or_null<MDTuple>(TParams),<br>
+                             cast_or_null<DISubprogram>(Decl),<br>
+                             MDTuple::getTemporary(VMContext, None).release());<br>
<br>
   if (isDefinition)<br>
     AllSubprograms.push_back(Node);<br>
@@ -723,11 +731,12 @@ DIBuilder::createMethod(DIScope *Context<br>
          "Methods should have both a Context and a context that isn't "<br>
          "the compile unit.");<br>
   // FIXME: Do we want to use different scope/lines?<br>
-  auto *SP = DISubprogram::get(<br>
-      VMContext, DIScopeRef::get(cast<DIScope>(Context)), Name, LinkageName, F,<br>
-      LineNo, Ty, isLocalToUnit, isDefinition, LineNo,<br>
-      DITypeRef::get(VTableHolder), VK, VIndex, Flags, isOptimized, Fn,<br>
-      cast_or_null<MDTuple>(TParam), nullptr, nullptr);<br>
+  auto *SP = getSubprogram(/* IsDistinct = */ isDefinition, VMContext,<br>
+                           DIScopeRef::get(cast<DIScope>(Context)), Name,<br>
+                           LinkageName, F, LineNo, Ty, isLocalToUnit,<br>
+                           isDefinition, LineNo, DITypeRef::get(VTableHolder),<br>
+                           VK, VIndex, Flags, isOptimized, Fn,<br>
+                           cast_or_null<MDTuple>(TParam), nullptr, nullptr);<br>
<br>
   if (isDefinition)<br>
     AllSubprograms.push_back(SP);<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>