[PATCH] D110363: [DWARF][NFC] add ParentIdx and SiblingIdx to DWARFDebugInfoEntry for faster navigation.

[Greg Clayton via Phabricator via llvm-commits]

clayborg added a comment.

So the DWARFUnit class no longer needs to be involved in the getting the parent, sibling, child stuff if we end up setting ParentIdx and SiblingIdx as a relative offset. LLDB's DWARF parser stores, and yes has a bad name for, the parent index which is the offset to subtract from "this" where "this" is a DWARFDebugInfoEntry. Since the DWARFUnits store an vector of DWARFDebugInfoEntry items after it parses all of the DIEs, then you can just subtract "ParentIdx" (which might be better named "ParentOffset") from "this" and get the correct DWARFDebugInfoEntry. Same with the SiblingIdx, if it is non zero, it is the offset to add to "this" to get the sibling. See inlined comments and see LLDB's DWARFDebugInfoEntry.h/.cpp and DWARFDie.h/.cpp.

The other thing to note is LLDB doesn't store the NULL tags in this DWARFDebugInfoEntry vector in the DWARFUnit. This saves a lot of memory for us, but that is for another patch.



================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugInfoEntry.h:28
+  /// Index of the parent die. UINT32_MAX if there is no parent.
+  uint32_t ParentIdx = UINT32_MAX;
+
----------------
LLDB actually stores this as the offset from this. So you can easily just do subtract math with "this" when you have a DWARFDebugInfoEntry since we know it is stored in an array. LLDB comments from it's DWARFDebugInfoEntry.h:
```
// How many to subtract from "this" to get the parent. If zero this die has no parent
```


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugInfoEntry.h:30
+
+  /// Index of the sibling die. Zero if there is no sibling.
+  uint32_t SiblingIdx = 0;
----------------
Same deal where sibling index is actually the number to add to "this" to get the sibling DIE.

```
// How many to add to "this" to get the sibling.
// If it is zero, then the DIE doesn't have children, or the
// DWARF claimed it had children but the DIE only contained
// a single NULL terminating child.
````


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugInfoEntry.h:47
   uint64_t getOffset() const { return Offset; }
-  uint32_t getDepth() const { return Depth; }
+
+  /// Returns index of the parent die.
----------------
If we store this as an offset for the parent index and sibling index, we can add simple functions here:

```
  DWARFDebugInfoEntry *getParent() {
    return ParentIdx > 0 ? this - ParentIdx : nullptr;
  }

  const DWARFDebugInfoEntry *getSibling() const {
    return SiblingIdx > 0 ? this + SiblingIdx : nullptr;
  }

  DWARFDebugInfoEntry *getFirstChild() {
    return hasChildren() ? this + 1 : nullptr;
  }
```
And then these functions can be used in the DWARFDie class.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:769-780
 DWARFDie DWARFUnit::getParent(const DWARFDebugInfoEntry *Die) {
   if (!Die)
     return DWARFDie();
-  const uint32_t Depth = Die->getDepth();
-  // Unit DIEs always have a depth of zero and never have parents.
-  if (Depth == 0)
-    return DWARFDie();
-  // Depth of 1 always means parent is the compile/type unit.
-  if (Depth == 1)
-    return getUnitDIE();
-  // Look for previous DIE with a depth that is one less than the Die's depth.
-  const uint32_t ParentDepth = Depth - 1;
-  for (uint32_t I = getDIEIndex(Die) - 1; I > 0; --I) {
-    if (DieArray[I].getDepth() == ParentDepth)
-      return DWARFDie(this, &DieArray[I]);
+
+  if (Optional<uint32_t> ParentIdx = Die->getParentIdx()) {
+    assert(*ParentIdx < DieArray.size() &&
+           "ParentIdx is out of DieArray boundaries");
----------------
This entire function is no longer needed if we use the suggested functions in DWARFDebugInfoEntry


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:782-797
 DWARFDie DWARFUnit::getSibling(const DWARFDebugInfoEntry *Die) {
   if (!Die)
     return DWARFDie();
-  uint32_t Depth = Die->getDepth();
-  // Unit DIEs always have a depth of zero and never have siblings.
-  if (Depth == 0)
-    return DWARFDie();
+
   // NULL DIEs don't have siblings.
   if (Die->getAbbreviationDeclarationPtr() == nullptr)
     return DWARFDie();
----------------
Ditto


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:799
 
 DWARFDie DWARFUnit::getPreviousSibling(const DWARFDebugInfoEntry *Die) {
   if (!Die)
----------------
This function could be done down in DWARFDebugInfoEntry now.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:828
 
 DWARFDie DWARFUnit::getFirstChild(const DWARFDebugInfoEntry *Die) {
   if (!Die->hasChildren())
----------------
This function could be done down in DWARFDebugInfoEntry now.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:839
 
 DWARFDie DWARFUnit::getLastChild(const DWARFDebugInfoEntry *Die) {
   if (!Die->hasChildren())
----------------
This function could be done down in DWARFDebugInfoEntry now.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:851-872
+  uint32_t DieIdx = getDIEIndex(Die);
+  if (DieIdx == 0 && DieArray.size() > 1 &&
+      DieArray.back().getTag() == dwarf::DW_TAG_null) {
+    // For the unit die we might take last item from DieArray.
+    assert(DieIdx == getDIEIndex(getUnitDIE()) && "Bad unit die");
+    return DWARFDie(this, &DieArray.back());
+  }
----------------
This code should no longer be needed right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110363/new/

https://reviews.llvm.org/D110363