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

[Greg Clayton via Phabricator via llvm-commits]

clayborg added a comment.

I am fine with keeping stuff in DWARFUnit if you want the extra asserts in the code.



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:786-788
   // NULL DIEs don't have siblings.
   if (Die->getAbbreviationDeclarationPtr() == nullptr)
     return DWARFDie();
----------------
We don't need this if statement anymore as the sibling index won't be set for NULL DIEs.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:799
 
 DWARFDie DWARFUnit::getPreviousSibling(const DWARFDebugInfoEntry *Die) {
   if (!Die)
----------------
clayborg wrote:
> This function could be done down in DWARFDebugInfoEntry now.
This is a complex function. Do we even use it? I would doubt that anyone ever asks for the previous DIE or uses a reverse iterator. It would be nice to remove this and the operator-- from the iterator, and the reverse_iterator for the DWARFDie and see if anyone is actually using it besides the unit test that tests it. IF we don't use it we can get rid of this and stop having to modify it if we update how DWARFDebugInfoEntry stores its data.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:859-871
+  uint32_t ChildIdx = DieIdx + 1;
+  while (ChildIdx < DieArray.size()) {
+    assert(*DieArray[ChildIdx].getParentIdx() == DieIdx && "Bad parent");
+
+    if (DieArray[ChildIdx].getTag() == dwarf::DW_TAG_null)
+      return DWARFDie(this, &DieArray[ChildIdx]);
+
----------------
What is this last while loop for? We should only need the normal case where SiblingIdx is set, or we have the unit die right?  Everything should have a sibling except the unit DIE. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110363