[PATCH] D154793: [DWARFLinkerParallel] Add support of accelerator tables to DWARFLinkerParallel.

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp:422
     Asm->OutStreamer->AddComment("Compilation unit " + Twine(CU.index()));
-    Asm->emitDwarfSymbolReference(CU.value());
+    if (std::holds_alternative<MCSymbol *>(CU.value()))
+      Asm->emitDwarfSymbolReference(std::get<MCSymbol *>(CU.value()));
----------------
Very cool


================
Comment at: llvm/lib/DWARFLinkerParallel/DIEAttributeCloner.cpp:229
   std::optional<std::pair<CompileUnit *, uint32_t>> RefDiePair =
-      CU.resolveDIEReference(Val);
+      CU.resolveDIEReference(Val, true);
   if (!RefDiePair) {
----------------
Maybe add an inline comment `/*CanResolveInterCUReferences=*/` or we can use an enum like this:

```
enum ResolveInterCUReferences : bool {
  CanResolveInterCUReferences = true,
  CannotResolveInterCUReferences = false,
};
```


================
Comment at: llvm/lib/DWARFLinkerParallel/DIEAttributeCloner.h:23-28
+  AttributesInfo() {
+    HasLiveAddress = false;
+    IsDeclaration = false;
+    HasRanges = false;
+    HasStringOffsetBaseAttr = false;
+  }
----------------
Can we continue to do the initialization inline? 


================
Comment at: llvm/lib/DWARFLinkerParallel/DWARFLinkerCompileUnit.cpp:1436
+  // DWARF.
+  size_t MaxNameDepth = 10;
+  size_t CurNameDepth = 0;
----------------
10 seems relatively small. What about something like 100 or 1000? Its only purpose is avoiding infinite recursion, right? Might as well pick something that's basically impossible to hit. 


================
Comment at: llvm/lib/DWARFLinkerParallel/DWARFLinkerCompileUnit.cpp:1467
+      ParentDie.getTag() == dwarf::DW_TAG_compile_unit ||
+      // FIXME: dsymutil-classic compatibility. Ignore modules.
+      ParentDie.getTag() == dwarf::DW_TAG_module)
----------------
FWIW, I don't think we need to keep any dsymutil-classic compatibility in the parallel linker. There's no way it's going to generate bit-identical output so there's no value in keeping bug-for-bug compat. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154793