[PATCH] D154793: [DWARFLinkerParallel] Add support of accelerator tables to DWARFLinkerParallel.
Jonas Devlieghere via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 21 12:04:50 PDT 2023
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
More information about the llvm-commits
mailing list