[llvm] r246098 - DI: Make Subprogram definitions 'distinct'

[David Blaikie via llvm-commits]

On Wed, Aug 26, 2015 at 3:50 PM, Duncan P. N. Exon Smith via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: dexonsmith
> Date: Wed Aug 26 17:50:16 2015
> New Revision: 246098
>
> URL: http://llvm.org/viewvc/llvm-project?rev=246098&view=rev
> Log:
> DI: Make Subprogram definitions 'distinct'
>
> Change `DIBuilder` always to produce 'distinct' nodes when creating
> `DISubprogram` definitions.  I measured a ~5% memory improvement in the
> link step (of ld64) when using `-flto -g`.
>
> `DISubprogram`s are used in two ways in the debug info graph.
>
> Some are definitions, point at actual functions, and can't really be
> shared between compile units.  With full debug info, these point down at
> their variables, forming uniquing cycles.  These uniquing cycles are
> expensive to link between modules, since all unique nodes that reference
> them transitively need to be duplicated (see commit message for r244181
> for more details).
>

*reads r244181*

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)

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.

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...


>
> Others are declarations, primarily used for member functions in the type
> hierarchy.  Definitions never show up there; instead, a definition
> points at its corresponding declaration node.
>
> I started by making all subprograms 'distinct'.  However, that was too
> big a hammer: memory usage *increased* ~5% (net increase vs. this patch
> of ~10%) because the 'distinct' declarations undermine LTO type
> uniquing.  This is a targeted fix for the definitions (where uniquing is
> an observable problem).
>
> A couple of notes:
>
>   - There's an accompanying commit to update IRGen testcases in clang.
>   - ^ That's what I'm using to test this commit.
>   - In a follow-up, I'll change the verifier to require 'distinct' on
>     definitions and add an upgrade to `BitcodeReader`.
>
> Modified:
>     llvm/trunk/lib/IR/DIBuilder.cpp
>
> Modified: llvm/trunk/lib/IR/DIBuilder.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=246098&r1=246097&r2=246098&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/IR/DIBuilder.cpp (original)
> +++ llvm/trunk/lib/IR/DIBuilder.cpp Wed Aug 26 17:50:16 2015
> @@ -675,6 +675,13 @@ DISubprogram *DIBuilder::createFunction(
>                          Flags, isOptimized, Fn, TParams, Decl);
>  }
>
> +template <class... Ts>
> +static DISubprogram *getSubprogram(bool IsDistinct, Ts &&... Args) {
> +  if (IsDistinct)
> +    return DISubprogram::getDistinct(std::forward<Ts>(Args)...);
> +  return DISubprogram::get(std::forward<Ts>(Args)...);
> +}
> +
>  DISubprogram *DIBuilder::createFunction(DIScope *Context, StringRef Name,
>                                          StringRef LinkageName, DIFile
> *File,
>                                          unsigned LineNo, DISubroutineType
> *Ty,
> @@ -684,12 +691,13 @@ DISubprogram *DIBuilder::createFunction(
>                                          MDNode *TParams, MDNode *Decl) {
>    assert(Ty->getTag() == dwarf::DW_TAG_subroutine_type &&
>           "function types should be subroutines");
> -  auto *Node = DISubprogram::get(
> -      VMContext, DIScopeRef::get(getNonCompileUnitScope(Context)), Name,
> -      LinkageName, File, LineNo, Ty, isLocalToUnit, isDefinition,
> ScopeLine,
> -      nullptr, 0, 0, Flags, isOptimized, Fn,
> cast_or_null<MDTuple>(TParams),
> -      cast_or_null<DISubprogram>(Decl),
> -      MDTuple::getTemporary(VMContext, None).release());
> +  auto *Node = getSubprogram(/* IsDistinct = */ isDefinition, VMContext,
> +
>  DIScopeRef::get(getNonCompileUnitScope(Context)),
> +                             Name, LinkageName, File, LineNo, Ty,
> isLocalToUnit,
> +                             isDefinition, ScopeLine, nullptr, 0, 0,
> Flags,
> +                             isOptimized, Fn,
> cast_or_null<MDTuple>(TParams),
> +                             cast_or_null<DISubprogram>(Decl),
> +                             MDTuple::getTemporary(VMContext,
> None).release());
>
>    if (isDefinition)
>      AllSubprograms.push_back(Node);
> @@ -723,11 +731,12 @@ DIBuilder::createMethod(DIScope *Context
>           "Methods should have both a Context and a context that isn't "
>           "the compile unit.");
>    // FIXME: Do we want to use different scope/lines?
> -  auto *SP = DISubprogram::get(
> -      VMContext, DIScopeRef::get(cast<DIScope>(Context)), Name,
> LinkageName, F,
> -      LineNo, Ty, isLocalToUnit, isDefinition, LineNo,
> -      DITypeRef::get(VTableHolder), VK, VIndex, Flags, isOptimized, Fn,
> -      cast_or_null<MDTuple>(TParam), nullptr, nullptr);
> +  auto *SP = getSubprogram(/* IsDistinct = */ isDefinition, VMContext,
> +                           DIScopeRef::get(cast<DIScope>(Context)), Name,
> +                           LinkageName, F, LineNo, Ty, isLocalToUnit,
> +                           isDefinition, LineNo,
> DITypeRef::get(VTableHolder),
> +                           VK, VIndex, Flags, isOptimized, Fn,
> +                           cast_or_null<MDTuple>(TParam), nullptr,
> nullptr);
>
>    if (isDefinition)
>      AllSubprograms.push_back(SP);
>
>
> _______________________________________________
> 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/20150826/ae46272c/attachment.html>