Modify MCCAS debug info section layout.

With this change, the layout of the debug info section in MCCAS has been
changed. Now, if there is a DIE that can be split off into it's own
CAS block, we do not attach it to its parent CAS block as a reference,
it gets added as a neighbor instead. This reduces the dependencies
between CAS blocks and increases chances of deduplication if a
DIEDataRef doesn't dedupe, by making sure it's parent will still dedupe.

This change also re-writes the materialization of the debug info section
to be iterative, rather than recursive.

Author: rastogishubham

llvm/include/llvm/MCCAS/MCCASObjectV1.h

@@ -718,7 +718,7 @@ class DIEDataRef : public SpecificRef<DIEDataRef> {
 struct LoadedDIETopLevel {
   SmallVector<StringRef, 0> AbbrevEntries;
   DIEDistinctDataRef DistinctData;
-  DIEDataRef RootDIE;
+  DIEDedupeTopLevelRef RootDIE;
 };
 
 /// Helper function to load the relevant information from a DIETopLevelRef:

llvm/lib/MCCAS/MCCASObjectV1.cpp

@@ -1893,7 +1893,9 @@ struct DIEDataWriter : public DataWriter {
 
   /// Saves the main data stream and any children to a new DIEDataRef node.
   Expected<DIEDataRef> getCASNode(MCCASBuilder &CASBuilder) {
-    return DIEDataRef::create(CASBuilder, Children, Data);
+    auto Ref = DIEDataRef::create(CASBuilder, Children, Data);
+    Data.clear();
+    return Ref;
   }
 
 private:
@@ -1973,13 +1975,15 @@ struct DIEToCASConverter {
   ArrayRef<char> DebugInfoData;
   MCCASBuilder &CASBuilder;
 
-  Expected<DIEDataRef> convertInNewDIEBlock(DWARFDie DIE,
-                                            DistinctDataWriter &DistinctWriter,
-                                            AbbrevSetWriter &AbbrevWriter);
+  Error convertInNewDIEBlock(
+      DWARFDie DIE, DistinctDataWriter &DistinctWriter,
+      AbbrevSetWriter &AbbrevWriter,
+      SmallVectorImpl<std::unique_ptr<DIEDataWriter>> &DIEWriters);
 
-  Error convertImpl(DWARFDie &DIE, DIEDataWriter &DIEWriter,
-                    DistinctDataWriter &DistinctWriter,
-                    AbbrevSetWriter &AbbrevWriter);
+  Error
+  convertImpl(DWARFDie &DIE, DIEDataWriter &DIEWriter,
+              DistinctDataWriter &DistinctWriter, AbbrevSetWriter &AbbrevWriter,
+              SmallVectorImpl<std::unique_ptr<DIEDataWriter>> &DIEWriters);
 };
 
 Error InMemoryCASDWARFObject::partitionCUData(ArrayRef<char> DebugInfoData,
@@ -2998,9 +3002,10 @@ static void writeDIEAttrs(DWARFDie &DIE, ArrayRef<char> DebugInfoData,
 /// * Otherwise, abbreviation_index is an index into the list of references of a
 /// DIEAbbrevSetRef block. In this case, raw_data should be interpreted
 /// according to the corresponding DIEAbbrevRefs block.
-Error DIEToCASConverter::convertImpl(DWARFDie &DIE, DIEDataWriter &DIEWriter,
-                                     DistinctDataWriter &DistinctWriter,
-                                     AbbrevSetWriter &AbbrevWriter) {
+Error DIEToCASConverter::convertImpl(
+    DWARFDie &DIE, DIEDataWriter &DIEWriter, DistinctDataWriter &DistinctWriter,
+    AbbrevSetWriter &AbbrevWriter,
+    SmallVectorImpl<std::unique_ptr<DIEDataWriter>> &DIEWriters) {
   Expected<unsigned> MaybeAbbrevIndex =
       AbbrevWriter.createAbbrevEntry(DIE, CASBuilder);
   if (!MaybeAbbrevIndex)
@@ -3020,39 +3025,40 @@ Error DIEToCASConverter::convertImpl(DWARFDie &DIE, DIEDataWriter &DIEWriter,
     // FIXME: don't use recursion.
     if (shouldCreateSeparateBlockFor(Child)) {
       DistinctWriter.writeULEB128(getDIEInAnotherBlockMarker());
-      auto MaybeNode =
-          convertInNewDIEBlock(Child, DistinctWriter, AbbrevWriter);
-      if (!MaybeNode)
-        return MaybeNode.takeError();
-      DIEWriter.addRef(*MaybeNode);
+      if (auto E = convertInNewDIEBlock(Child, DistinctWriter, AbbrevWriter,
+                                        DIEWriters))
+        return E;
       continue;
     }
-
-    if (auto E = convertImpl(Child, DIEWriter, DistinctWriter, AbbrevWriter))
+    if (auto E = convertImpl(Child, DIEWriter, DistinctWriter, AbbrevWriter,
+                             DIEWriters))
       return E;
   }
   return Error::success();
 }
 
-Expected<DIEDataRef>
-DIEToCASConverter::convertInNewDIEBlock(DWARFDie DIE,
-                                        DistinctDataWriter &DistinctWriter,
-                                        AbbrevSetWriter &AbbrevWriter) {
-  DIEDataWriter DIEWriter;
-  if (auto E = convertImpl(DIE, DIEWriter, DistinctWriter, AbbrevWriter))
-    return std::move(E);
-  return DIEWriter.getCASNode(CASBuilder);
+Error DIEToCASConverter::convertInNewDIEBlock(
+    DWARFDie DIE, DistinctDataWriter &DistinctWriter,
+    AbbrevSetWriter &AbbrevWriter,
+    SmallVectorImpl<std::unique_ptr<DIEDataWriter>> &DIEWriters) {
+  auto DIEWriter = std::make_unique<DIEDataWriter>();
+  DIEWriters.push_back(std::move(DIEWriter));
+  if (auto E = convertImpl(DIE, *DIEWriters.back(), DistinctWriter,
+                           AbbrevWriter, DIEWriters))
+    return E;
+  return Error::success();
 }
 
 Expected<DIETopLevelRef>
 DIEToCASConverter::convert(DWARFDie DIE, ArrayRef<char> HeaderData,
                            AbbrevSetWriter &AbbrevWriter) {
   DistinctDataWriter DistinctWriter;
   DistinctWriter.writeData(HeaderData);
-  Expected<DIEDataRef> MaybeDIE =
-      convertInNewDIEBlock(DIE, DistinctWriter, AbbrevWriter);
-  if (!MaybeDIE)
-    return MaybeDIE.takeError();
+  SmallVector<std::unique_ptr<DIEDataWriter>> DIEWriters;
+  if (Error E =
+          convertInNewDIEBlock(DIE, DistinctWriter, AbbrevWriter, DIEWriters))
+    return std::move(E);
+
   Expected<DIEAbbrevSetRef> MaybeAbbrevSet =
       AbbrevWriter.endAbbrevSet(CASBuilder);
   if (!MaybeAbbrevSet)
@@ -3061,8 +3067,21 @@ DIEToCASConverter::convert(DWARFDie DIE, ArrayRef<char> HeaderData,
       DistinctWriter.getCASNode(CASBuilder);
   if (!MaybeDistinct)
     return MaybeDistinct.takeError();
+
+  SmallVector<cas::ObjectRef> DIERefs;
+  DIERefs.reserve(DIEWriters.size());
+  for (auto &Writer : DIEWriters) {
+    Expected<DIEDataRef> DIERef = Writer->getCASNode(CASBuilder);
+    if (!DIERef)
+      return DIERef.takeError();
+    DIERefs.push_back(DIERef->getRef());
+  }
+
+  auto TopDIERef = DIEDedupeTopLevelRef::create(CASBuilder, DIERefs);
+  if (!TopDIERef)
+    return TopDIERef.takeError();
   SmallVector<cas::ObjectRef, 3> Refs{
-      MaybeDIE->getRef(), MaybeAbbrevSet->getRef(), MaybeDistinct->getRef()};
+      TopDIERef->getRef(), MaybeAbbrevSet->getRef(), MaybeDistinct->getRef()};
   return DIETopLevelRef::create(CASBuilder, Refs);
 }
 
@@ -3074,8 +3093,8 @@ mccasformats::v1::loadDIETopLevel(DIETopLevelRef TopLevelRef) {
         "TopLevelRef is expected to have three references");
 
   const MCSchema &Schema = TopLevelRef.getSchema();
-  Expected<DIEDataRef> RootDIE =
-      DIEDataRef::get(Schema, TopLevelRef.getReference(0));
+  Expected<DIEDedupeTopLevelRef> RootDIE =
+      DIEDedupeTopLevelRef::get(Schema, TopLevelRef.getReference(0));
   Expected<DIEAbbrevSetRef> AbbrevSet =
       DIEAbbrevSetRef::get(Schema, TopLevelRef.getReference(1));
   Expected<DIEDistinctDataRef> DistinctData =
@@ -3097,9 +3116,8 @@ mccasformats::v1::loadDIETopLevel(DIETopLevelRef TopLevelRef) {
 }
 
 struct DIEVisitor {
-  Error visitDIERef(DIEDataRef Ref);
-  Error visitDIERef(BinaryStreamReader &DataReader, unsigned AbbrevIdx,
-                    StringRef DIEData, ArrayRef<DIEDataRef> &DIEChildrenStack);
+  Error visitDIERef(DIEDedupeTopLevelRef Ref);
+  Error visitDIERef(ArrayRef<DIEDataRef> &DIEChildrenStack);
   Error visitDIEAttrs(AbbrevEntryReader &AbbrevReader,
                       BinaryStreamReader &Reader, StringRef DIEData);
 
@@ -3165,67 +3183,99 @@ static AbbrevEntryReader getAbbrevEntryReader(ArrayRef<StringRef> AbbrevEntries,
   return AbbrevEntryReader(AbbrevData);
 }
 
-Error DIEVisitor::visitDIERef(BinaryStreamReader &DataReader,
-                              unsigned AbbrevIdx, StringRef DIEData,
-                              ArrayRef<DIEDataRef> &DIEChildrenStack) {
-  AbbrevEntryReader AbbrevReader =
-      getAbbrevEntryReader(AbbrevEntries, AbbrevIdx);
-
-  if (Expected<dwarf::Tag> MaybeTag = AbbrevReader.readTag())
-    StartTagCallback(*MaybeTag, AbbrevIdx);
-  else
-    return MaybeTag.takeError();
+/// Restores the state of the \p Reader and \p Data
+/// arguments to a previous state. The algorithm in visitDIERefs is an iterative
+/// implementation of a Depth First Search, and this function is used to
+/// simulate a return from a recursive callback, by restoring the locals to a
+/// previous stack frame.
+static void popStack(BinaryStreamReader &Reader, StringRef &Data,
+                     std::stack<std::pair<StringRef, unsigned>> &StackOfNodes) {
+  auto DataAndOffset = StackOfNodes.top();
+  Reader = BinaryStreamReader(DataAndOffset.first, llvm::endianness::little);
+  Data = DataAndOffset.first;
+  Reader.setOffset(DataAndOffset.second);
+  StackOfNodes.pop();
+}
+
+// Visit DIERef CAS objects and materialize them.
+Error DIEVisitor::visitDIERef(ArrayRef<DIEDataRef> &DIEChildrenStack) {
+
+  std::stack<std::pair<StringRef, unsigned>> StackOfNodes;
+  auto Data = DIEChildrenStack.empty() ? StringRef()
+                                       : DIEChildrenStack.front().getData();
+  BinaryStreamReader Reader(Data, llvm::endianness::little);
+
+  while (!DistinctReader.empty()) {
+
+    Expected<uint64_t> MaybeAbbrevIdx = readAbbrevIdx(DistinctReader);
+    if (!MaybeAbbrevIdx)
+      return MaybeAbbrevIdx.takeError();
+    auto AbbrevIdx = *MaybeAbbrevIdx;
+
+    // If we see a EndOfDIESiblingsMarker, we know that this sequence of
+    // Children has no more siblings and we need to pop the StackOfNodes and
+    // continue materialization of the parent's siblings that may exist.
+    if (AbbrevIdx == getEndOfDIESiblingsMarker()) {
+      EndTagCallback(true /*HadChildren*/);
+      if (!StackOfNodes.empty() && Reader.empty())
+        popStack(Reader, Data, StackOfNodes);
+      continue;
+    }
 
-  Expected<bool> MaybeHasChildren = AbbrevReader.readHasChildren();
-  if (!MaybeHasChildren)
-    return MaybeHasChildren.takeError();
+    // If we see a DIEInAnotherBlockMarker, we know that the next DIE is in
+    // another CAS Block, we have to push the current CAS Object on the stack,
+    // and materialize the next DIE from the DIEChildrenStack.
+    if (AbbrevIdx == getDIEInAnotherBlockMarker()) {
+      StackOfNodes.push(std::make_pair(Data, Reader.getOffset()));
+      DIEChildrenStack = DIEChildrenStack.drop_front();
+      Data = DIEChildrenStack.front().getData();
+      NewBlockCallback(DIEChildrenStack.front().getID().toString());
+      Reader = BinaryStreamReader(Data, llvm::endianness::little);
+      continue;
+    }
 
-  if (auto E = visitDIEAttrs(AbbrevReader, DataReader, DIEData))
-    return E;
+    // If we have a legitimate AbbrevIdx, materialize the current DIE.
+    AbbrevEntryReader AbbrevReader =
+        getAbbrevEntryReader(AbbrevEntries, AbbrevIdx);
 
-  if (!*MaybeHasChildren) {
-    EndTagCallback(false /*HadChildren*/);
-    return Error::success();
-  }
+    if (Expected<dwarf::Tag> MaybeTag = AbbrevReader.readTag())
+      StartTagCallback(*MaybeTag, AbbrevIdx);
+    else
+      return MaybeTag.takeError();
 
-  while (true) {
-    Expected<uint64_t> ChildAbbrevIdx = readAbbrevIdx(DistinctReader);
-    if (!ChildAbbrevIdx)
-      return ChildAbbrevIdx.takeError();
+    Expected<bool> MaybeHasChildren = AbbrevReader.readHasChildren();
+    if (!MaybeHasChildren)
+      return MaybeHasChildren.takeError();
 
-    if (*ChildAbbrevIdx == getEndOfDIESiblingsMarker())
-      break;
+    if (auto E = visitDIEAttrs(AbbrevReader, Reader, Data))
+      return E;
 
-    if (*ChildAbbrevIdx == getDIEInAnotherBlockMarker()) {
-      if (auto E = visitDIERef(DIEChildrenStack.front()))
-        return E;
-      DIEChildrenStack = DIEChildrenStack.drop_front();
-      continue;
+    // If the current DIE doesn't have any children, the current CAS Object will
+    // not contain any more data, pop the stack to continue materializing its
+    // parent's siblings that may exist.
+    if (!*MaybeHasChildren) {
+      if (!StackOfNodes.empty() && Reader.empty())
+        popStack(Reader, Data, StackOfNodes);
+      EndTagCallback(false /*HadChildren*/);
     }
-
-    if (auto E =
-            visitDIERef(DataReader, *ChildAbbrevIdx, DIEData, DIEChildrenStack))
-      return E;
   }
-
-  EndTagCallback(true /*HadChildren*/);
   return Error::success();
 }
 
-Error DIEVisitor::visitDIERef(DIEDataRef StartDIERef) {
-  StringRef DIEData = StartDIERef.getData();
-  BinaryStreamReader DataReader(DIEData, endianness::little);
+Error DIEVisitor::visitDIERef(DIEDedupeTopLevelRef StartDIERef) {
 
+  auto Offset = DistinctReader.getOffset();
   Expected<uint64_t> MaybeAbbrevIdx = readAbbrevIdx(DistinctReader);
   if (!MaybeAbbrevIdx)
     return MaybeAbbrevIdx.takeError();
   auto AbbrevIdx = *MaybeAbbrevIdx;
-
   // The tag of a fresh block must be meaningful, otherwise we wouldn't have
   // made a new block.
   assert(AbbrevIdx != getEndOfDIESiblingsMarker() &&
          AbbrevIdx != getDIEInAnotherBlockMarker());
 
+  DistinctReader.setOffset(Offset);
+
   NewBlockCallback(StartDIERef.getID().toString());
 
   Expected<SmallVector<DIEDataRef>> MaybeChildren =
@@ -3234,7 +3284,7 @@ Error DIEVisitor::visitDIERef(DIEDataRef StartDIERef) {
     return MaybeChildren.takeError();
   ArrayRef<DIEDataRef> Children = *MaybeChildren;
 
-  return visitDIERef(DataReader, AbbrevIdx, DIEData, Children);
+  return visitDIERef(Children);
 }
 
 Error mccasformats::v1::visitDebugInfo(

llvm/test/tools/llvm-cas-dump/basic_debug_test.ll

@@ -28,6 +28,7 @@
 ; CHECK-NEXT:       mc:padding  llvmcas://{{.*}}
 ; CHECK-NEXT:     mc:debug_info_section  llvmcas://{{.*}}
 ; CHECK-NEXT:       mc:debug_DIE_top_level  llvmcas://{{.*}}
+; CHECK-NEXT:         mc:debug_DIE_Dedupe_top_level  llvmcas://{{.*}}
 ; CHECK-NEXT:         mc:debug_DIE_data       llvmcas://{{.*}}
 ; CHECK-NEXT:         mc:debug_DIE_data       llvmcas://{{.*}}
 ; CHECK-NEXT:         mc:debug_DIE_abbrev_set llvmcas://{{.*}}