[PATCH] Refactor debug info lexical block generation

[David Blaikie]

On Wed, May 27, 2015 at 11:32 AM, Aboud, Amjad <amjad.aboud at intel.com>
wrote:

>  I am not sure what is imported entry is, and if it could be located
> inside a lexical block.
>
>
>
> When you create an imported entry you call the “getOrCreateContextDIE”,
> this function will check for, Compile-Unit, File, Namespace, Type,
> Subprogram context.
>
> If none match, it calls “getDIE” function:
>
>
>
> DIE *DwarfUnit::getDIE(const DINode *D) const {
>
>   if (isShareableAcrossCUs(D))
>
>     return DU->getDIE(D);
>
>   return MDNodeToDieMap.lookup(D);
>
> }
>
>
>
> Maybe imported entries fit with the first condition check of “
> isShareableAcrossCUs”, but the function static variables will have a
> lexical block context and when we will look for this context in the “
> MDNodeToDieMap” map it will not be there, because we do not map lexical
> block MDNode to lexical block DIE.
>

Take a look in DwarfCompileUnit::cosntructScopeDIE, around half way
through, you'll find this code:

    unsigned ChildScopeCount;

    // We create children here when we know the scope DIE is not going to be
    // null and the children will be added to the scope DIE.
    createScopeChildrenDIE(Scope, Children, &ChildScopeCount);

    // 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)));
    }

<<It seems to me like the code there that the logic for static variables
should go here too - the same way imported entities are dealt with. Then,
if we have any static variables, imported entities, we create this scope -
otherwise we put it in the parent>>

    // If there are only other scopes as children, put them directly in the
    // parent instead, as this scope would serve no purpose.
    if (Children.size() == ChildScopeCount) {
      FinalChildren.insert(FinalChildren.end(),
                           std::make_move_iterator(Children.begin()),
                           std::make_move_iterator(Children.end()));
      return;
    }
    ScopeDIE = constructLexicalScopeDIE(Scope);
    assert(ScopeDIE && "Scope DIE should not be null.");




>
>
> Also, right now, we decide if to create the lexical block DIE or not
> depends on the local variable children only, as the function static
> variables DIE and the type DIEs were not attached yet to the lexical block
> DIE.
>
> This, is why I also suggest to create all lexical blocks at first and once
> all its children are created, we can revisit this lexical block FIE and
> decide if it worth collapsing with parent.
>
>
>
> Regards,
>
> Amjad
>
>
>
>
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> *Sent:* Wednesday, May 27, 2015 21:14
> *To:* reviews+D9960+public+41c324f590718ccc at reviews.llvm.org
> *Cc:* Aboud, Amjad; Eric Christopher; Benjamin Kramer;
> llvm-commits at cs.uiuc.edu; Robinson, Paul
> *Subject:* Re: [PATCH] Refactor debug info lexical block generation
>
>
>
> Perhaps it'd be good to have a general design discussion here, rather than
> in the form of line-by-line code review. A few broad issues have been
> brought up by both Paul and Frederic that sound reasonable to me, though
> I've not looked at the code in detail yet.
>
>
> Is there any reason this wouldn't be implemented the same way as the
> imported entity work? Where we check for imported entities at scope
> construction time to decide whether to create that scope or not.
>
>
>
> On Sun, May 24, 2015 at 9:00 AM, Amjad Aboud <amjad.aboud at intel.com>
> wrote:
>
> Hi echristo, bkramer, dblaikie,
>
> This patch is needed for resolving Bug 19238.
> It is the first part which will be followed by D9758.
>
> The idea is to create all debug info lexical block DIEs and add them to
> the DIScope->DIE map.
> Latter, just before emitting the DIEs, we will search the DIE tree and
> collapse all redundant (useless) DIE lexical blocks.
>
> REPOSITORY
>   rL LLVM
>
> http://reviews.llvm.org/D9960
>
> Files:
>   include/llvm/CodeGen/DIE.h
>   lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>   lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>   lib/CodeGen/AsmPrinter/DwarfDebug.h
>   test/DebugInfo/useless_lexical_scope.ll
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150527/59797377/attachment.html>