[PATCH] [DebugInfo] Add a getParent method to DWARFDebugInformationEntryMinimal.

[David Blaikie]

On Fri, Jan 16, 2015 at 4:16 PM, Frederic Riss <friss at apple.com> wrote:

> Hi samsonov, dblaikie,
>
> The DIE representation of libDebugInfo is designed to be minimal
> (as the class name implies). The information that's there allows
> to walk the DIE tree down, but you can't get back to the root
> from an arbitrary node.
>
> This patch adds this ability with the goal of using it in
> llvm-dsymutil. I could also use this information in dwarfdump
> itself to easily support dumping the full DIE hierarchy of a
> selected DIE when I finally implement a search option.
>
> The added information definitely has a cost: the size of
> the structure is increased by 8 bytes on 64 bits archs
> due to padding and this combined to the added logic makes
> the raw parsing between 5 and 10% slower.
>
> Just for llvm-dsymutil, this is optional. I can gather the
> parent chains in a separate path and store them, but I think
> that the performance/aded functionality benefit is worth
> it. WDYT?
>

I doubt the performance of dwarfdump is terribly important - but I would
imagine the performance of dsymutil will be/is, so it might be worth
looking at the alternatives that don't increase the memory cost of all DIEs?

(I imagine a search query that responds with a path, rather than a node,
would be a reasonable API, for example - but I dunno)


>
> http://reviews.llvm.org/D7025
>
> Files:
>   include/llvm/DebugInfo/DWARFDebugInfoEntry.h
>   lib/DebugInfo/DWARFUnit.cpp
>
> Index: include/llvm/DebugInfo/DWARFDebugInfoEntry.h
> ===================================================================
> --- include/llvm/DebugInfo/DWARFDebugInfoEntry.h
> +++ include/llvm/DebugInfo/DWARFDebugInfoEntry.h
> @@ -30,13 +30,16 @@
>    /// Offset within the .debug_info of the start of this entry.
>    uint32_t Offset;
>
> +  /// How many to substract to "this" to get the parent.
> +  uint32_t ParentIdx;
> +
>    /// How many to add to "this" to get the sibling.
>    uint32_t SiblingIdx;
>
>    const DWARFAbbreviationDeclaration *AbbrevDecl;
>  public:
>    DWARFDebugInfoEntryMinimal()
> -    : Offset(0), SiblingIdx(0), AbbrevDecl(nullptr) {}
> +    : Offset(0), ParentIdx(0), SiblingIdx(0), AbbrevDecl(nullptr) {}
>
>    void dump(raw_ostream &OS, DWARFUnit *u, unsigned recurseDepth,
>              unsigned indent = 0) const;
> @@ -66,6 +69,11 @@
>      return SiblingIdx > 0 ? this + SiblingIdx : nullptr;
>    }
>
> +  // Returns the parent DIE or null if this is the root DIE.
> +  const DWARFDebugInfoEntryMinimal *getParent() const {
> +    return ParentIdx ? this - ParentIdx : nullptr;
> +  }
> +
>    // We know we are kept in a vector of contiguous entries, so we know
>    // we don't need to store our child pointer, if we have a child it will
>    // be the next entry in the list...
> @@ -82,6 +90,15 @@
>        SiblingIdx = 0;
>    }
>
> +  void setParent(const DWARFDebugInfoEntryMinimal *Parent) {
> +    if (Parent) {
> +      // We know we are kept in a vector of contiguous entries, so we know
> +      // our parent will be soewhere before "this".
> +      ParentIdx = this - Parent;
> +    } else
> +      ParentIdx = 0;
> +  }
> +
>    const DWARFAbbreviationDeclaration *getAbbreviationDeclarationPtr()
> const {
>      return AbbrevDecl;
>    }
> Index: lib/DebugInfo/DWARFUnit.cpp
> ===================================================================
> --- lib/DebugInfo/DWARFUnit.cpp
> +++ lib/DebugInfo/DWARFUnit.cpp
> @@ -143,12 +143,18 @@
>    if (DieArray.size() <= 1)
>      return;
>
> -  std::vector<DWARFDebugInfoEntryMinimal *> ParentChain;
> +  // Make sure the vector is never empty so that we don't have to
> +  // check for emptyness inside the loop bellow.
> +  std::vector<DWARFDebugInfoEntryMinimal *> ParentChain(1,&DieArray[0]);
>    DWARFDebugInfoEntryMinimal *SiblingChain = nullptr;
>    for (auto &DIE : DieArray) {
>      if (SiblingChain) {
>        SiblingChain->setSibling(&DIE);
>      }
> +    // Note that this will call setParent with itself on the root
> +    // node. This will result in a null offset which is handled as 'no
> +    // parent'.
> +    DIE.setParent(ParentChain.back());
>      if (const DWARFAbbreviationDeclaration *AbbrDecl =
>              DIE.getAbbreviationDeclarationPtr()) {
>        // Normal DIE.
> @@ -165,7 +171,7 @@
>      }
>    }
>    assert(SiblingChain == nullptr || SiblingChain == &DieArray[0]);
> -  assert(ParentChain.empty());
> +  assert(ParentChain.size() == 1 && ParentChain[0] == &DieArray[0]);
>  }
>
>  void DWARFUnit::extractDIEsToVector(
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150117/25e5e5cd/attachment.html>