[DebugInfo][DwarfDebug][CodeView] Allow DISubprogram to be attached to multiple Functions #162854
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dzhidzhoev
requested review from
CarlosAlbertoEnciso,
OCHyams,
SLTozer,
adrian-prantl,
dwblaikie,
jmorse,
mstorsjo,
pogo59 and
rnk
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-debuginfo Author: Vladislav Dzhidzhoev (dzhidzhoev) ChangesDepends on:
In #75385 (and the following tries), an attempt was made, to support attaching local types to DILocalScopes, and to store function local types in DISubprogram's That patch failed to land due to issues arising during LTO process. If two definition DISubprograms from different compile units represent, essentially, the same source code function, and have common local DICompositeType, and if this DICompositeType is uniqued (due to ODRUniquingDebugTypes feature), the subprograms end up having wrong retainedNodes list/scoping relationship. To tackle this issue, in #142166, it was proposed to force-unique all DISubporgrams even if they don't contain odr-uniqued types (#142166 (comment)). It should establish one-to-one-to-many relationship between DISubprograms, abstract DIEs and function clones (from different CUs, in case of LTO). To implement that, AsmPrinter should support correct emission of debug info for DISubprograms attached to multiple functions. This is the goal of this commit. Here, LexicalScope's function map is changed to multimap between DISubprogram and (possible multiple) functions attached to it. LexicalScope is modified to create an abstract scope for a DISubprogram having multiple lllvm::Function attachments. CodeViewDebug is adopted as well. UDTs are ensured to be emmited properly in the cases that are addressed here. Please let me know if more changes to CodeView needed, as I'm not very familiar with the format. Patch is 56.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/162854.diff 20 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/LexicalScopes.h b/llvm/include/llvm/CodeGen/LexicalScopes.h
index 993df54c05ad5..848a4a4ce2433 100644
--- a/llvm/include/llvm/CodeGen/LexicalScopes.h
+++ b/llvm/include/llvm/CodeGen/LexicalScopes.h
@@ -199,12 +199,18 @@ class LexicalScopes {
return I != LexicalScopeMap.end() ? &I->second : nullptr;
}
+ bool currentFunctionHasInlinedScopes() {
+ return !InlinedLexicalScopeMap.empty();
+ }
+
/// Find or create an abstract lexical scope.
LLVM_ABI LexicalScope *getOrCreateAbstractScope(const DILocalScope *Scope);
- /// Get function to which the given subprogram is attached, if exists.
- const Function *getFunction(const DISubprogram *SP) const {
- return FunctionMap.lookup(SP);
+ /// Get functions to which the given subprogram is attached.
+ const SmallPtrSet<const Function *, 1> *
+ getFunctions(const DISubprogram *SP) const {
+ auto I = FunctionMap.find(SP);
+ return I == FunctionMap.end() ? nullptr : &I->second;
}
private:
@@ -218,6 +224,15 @@ class LexicalScopes {
: nullptr;
}
+ /// Create abstract lexical scope for local scopes used by multiple
+ /// functions, if needed.
+ void ensureAbstractLexicalScopeIsCreated(const DILocalScope *Scope) {
+ const DISubprogram *SP = Scope->getSubprogram();
+ const auto *Fns = getFunctions(SP);
+ if (!Fns || Fns->size() != 1)
+ getOrCreateAbstractScope(Scope);
+ }
+
/// Find or create a regular lexical scope.
LexicalScope *getOrCreateRegularScope(const DILocalScope *Scope);
@@ -237,7 +252,7 @@ class LexicalScopes {
const MachineFunction *MF = nullptr;
/// Mapping between DISubprograms and IR functions.
- DenseMap<const DISubprogram *, const Function *> FunctionMap;
+ DenseMap<const DISubprogram *, SmallPtrSet<const Function *, 1>> FunctionMap;
/// Tracks the scopes in the current function.
// Use an unordered_map to ensure value pointer validity over insertion.
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index 12d749ce56f06..9bc395fcf1e79 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -1658,9 +1658,9 @@ void CodeViewDebug::addToUDTs(const DIType *Ty) {
formatNestedName(ParentScopeNames, getPrettyScopeName(Ty));
if (ClosestSubprogram == nullptr) {
- GlobalUDTs.emplace_back(std::move(FullyQualifiedName), Ty);
+ GlobalUDTs[Ty] = std::move(FullyQualifiedName);
} else if (ClosestSubprogram == CurrentSubprogram) {
- LocalUDTs.emplace_back(std::move(FullyQualifiedName), Ty);
+ LocalUDTs[Ty] = std::move(FullyQualifiedName);
}
// TODO: What if the ClosestSubprogram is neither null or the current
@@ -2698,6 +2698,12 @@ TypeIndex CodeViewDebug::getTypeIndex(const DIType *Ty, const DIType *ClassTy) {
if (!Ty)
return TypeIndex::Void();
+ if (Ty->getTag() == dwarf::DW_TAG_typedef) {
+ // Ensure that UDT is added even if the (local) type has been translated
+ // during processing of the previous function.
+ addToUDTs(Ty);
+ }
+
// Check if we've already translated this type. Don't try to do a
// get-or-create style insertion that caches the hash lookup across the
// lowerType call. It will update the TypeIndices map.
@@ -2787,6 +2793,10 @@ TypeIndex CodeViewDebug::getCompleteTypeIndex(const DIType *Ty) {
return FwdDeclTI;
}
+ // Ensure that UDT is added even if the (local) type has been translated
+ // during processing of the previous function.
+ addToUDTs(CTy);
+
// Check if we've already translated the complete record type.
// Insert the type with a null TypeIndex to signify that the type is currently
// being lowered.
@@ -3217,20 +3227,19 @@ void CodeViewDebug::emitEndSymbolRecord(SymbolKind EndKind) {
OS.emitInt16(uint16_t(EndKind)); // Record Kind
}
-void CodeViewDebug::emitDebugInfoForUDTs(
- const std::vector<std::pair<std::string, const DIType *>> &UDTs) {
+template <typename Range>
+void CodeViewDebug::emitDebugInfoForUDTs(Range &&UDTs) {
#ifndef NDEBUG
size_t OriginalSize = UDTs.size();
#endif
- for (const auto &UDT : UDTs) {
- const DIType *T = UDT.second;
+ for (const auto &[T, Name] : UDTs) {
assert(shouldEmitUdt(T));
MCSymbol *UDTRecordEnd = beginSymbolRecord(SymbolKind::S_UDT);
OS.AddComment("Type");
OS.emitInt32(getCompleteTypeIndex(T).getIndex());
assert(OriginalSize == UDTs.size() &&
"getCompleteTypeIndex found new UDTs!");
- emitNullTerminatedSymbolName(OS, UDT.first);
+ emitNullTerminatedSymbolName(OS, Name);
endSymbolRecord(UDTRecordEnd);
}
}
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
index c2b878e52e1c3..f4c5a5bb1625c 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
@@ -306,8 +306,8 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
// The UDTs we have seen while processing types; each entry is a pair of type
// index and type name.
- std::vector<std::pair<std::string, const DIType *>> LocalUDTs;
- std::vector<std::pair<std::string, const DIType *>> GlobalUDTs;
+ MapVector<const DIType *, std::string> LocalUDTs;
+ MapVector<const DIType *, std::string> GlobalUDTs;
using FileToFilepathMapTy = std::map<const DIFile *, std::string>;
FileToFilepathMapTy FileToFilepathMap;
@@ -352,8 +352,7 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
void emitDebugInfoForRetainedTypes();
- void emitDebugInfoForUDTs(
- const std::vector<std::pair<std::string, const DIType *>> &UDTs);
+ template <typename Range> void emitDebugInfoForUDTs(Range &&UDTs);
void collectDebugInfoForGlobals();
void emitDebugInfoForGlobals();
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index 518121e200190..a801b1e400ad9 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -178,7 +178,7 @@ unsigned DwarfCompileUnit::getOrCreateSourceID(const DIFile *File) {
DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(
const DIGlobalVariable *GV, ArrayRef<GlobalExpr> GlobalExprs) {
// Check for pre-existence.
- if (DIE *Die = getDIE(GV))
+ if (DIE *Die = getDIEs(GV).getVariableDIE(GV))
return Die;
assert(GV);
@@ -795,7 +795,9 @@ DIE *DwarfCompileUnit::constructLexicalScopeDIE(LexicalScope *Scope) {
DIE *DwarfCompileUnit::constructVariableDIE(DbgVariable &DV, bool Abstract) {
auto *VariableDie = DIE::get(DIEValueAllocator, DV.getTag());
- insertDIE(DV.getVariable(), VariableDie);
+ getDIEs(DV.getVariable())
+ .getLVs()
+ .insertDIE(DV.getVariable(), &DV, VariableDie, Abstract);
DV.setDIE(*VariableDie);
// Abstract variables don't get common attributes later, so apply them now.
if (Abstract) {
@@ -1010,7 +1012,9 @@ DIE *DwarfCompileUnit::constructVariableDIE(DbgVariable &DV,
DIE *DwarfCompileUnit::constructLabelDIE(DbgLabel &DL,
const LexicalScope &Scope) {
auto LabelDie = DIE::get(DIEValueAllocator, DL.getTag());
- insertDIE(DL.getLabel(), LabelDie);
+ getDIEs(DL.getLabel())
+ .getLabels()
+ .insertDIE(DL.getLabel(), &DL, LabelDie, Scope.isAbstractScope());
DL.setDIE(*LabelDie);
if (Scope.isAbstractScope())
@@ -1472,8 +1476,9 @@ DIE *DwarfCompileUnit::getOrCreateImportedEntityDIE(
return IMDie;
}
-void DwarfCompileUnit::finishSubprogramDefinition(const DISubprogram *SP) {
- DIE *D = getDIE(SP);
+void DwarfCompileUnit::finishSubprogramDefinition(const DISubprogram *SP,
+ const Function *F) {
+ DIE *D = getDIEs(SP).getLocalScopes().getConcreteDIE(SP, F);
if (DIE *AbsSPDIE = getAbstractScopeDIEs().lookup(SP)) {
if (D)
// If this subprogram has an abstract definition, reference that
@@ -1834,14 +1839,16 @@ DIE *DwarfCompileUnit::getOrCreateSubprogramDIE(const DISubprogram *SP,
const Function *F,
bool Minimal) {
if (!F && SP->isDefinition()) {
- F = DD->getLexicalScopes().getFunction(SP);
+ const auto *Fs = DD->getLexicalScopes().getFunctions(SP);
- if (!F) {
+ if (!Fs || Fs->size() != 1) {
// SP may belong to another CU. Determine the CU similarly
// to DwarfDebug::constructAbstractSubprogramScopeDIE.
return &DD->getOrCreateAbstractSubprogramCU(SP, *this)
.getOrCreateAbstractSubprogramDIE(SP);
}
+
+ F = *Fs->begin();
}
return DwarfUnit::getOrCreateSubprogramDIE(SP, F, Minimal);
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
index a3bbc8364599d..b0dcc3e432a03 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
@@ -79,16 +79,10 @@ class DwarfCompileUnit final : public DwarfUnit {
// List of concrete lexical block scopes belong to subprograms within this CU.
DenseMap<const DILocalScope *, DIE *> LexicalBlockDIEs;
- // List of abstract local scopes (either DISubprogram or DILexicalBlock).
- DenseMap<const DILocalScope *, DIE *> AbstractLocalScopeDIEs;
- SmallPtrSet<const DISubprogram *, 8> FinalizedAbstractSubprograms;
-
// List of inlined lexical block scopes that belong to subprograms within this
// CU.
DenseMap<const DILocalScope *, SmallVector<DIE *, 2>> InlinedLocalScopeDIEs;
- DenseMap<const DINode *, std::unique_ptr<DbgEntity>> AbstractEntities;
-
/// DWO ID for correlating skeleton and split units.
uint64_t DWOId = 0;
@@ -126,22 +120,20 @@ class DwarfCompileUnit final : public DwarfUnit {
bool isDwoUnit() const override;
- DenseMap<const DILocalScope *, DIE *> &getAbstractScopeDIEs() {
- if (isDwoUnit() && !DD->shareAcrossDWOCUs())
- return AbstractLocalScopeDIEs;
- return DU->getAbstractScopeDIEs();
+ DwarfInfoHolder &getDIEs(const DINode *N) { return DwarfUnit::getDIEs(N); }
+
+ DwarfInfoHolder &getDIEs() { return getDIEs(nullptr); }
+
+ DwarfInfoHolder::AbstractScopeMapT &getAbstractScopeDIEs() {
+ return getDIEs().getLocalScopes().getAbstractDIEs();
}
DenseMap<const DINode *, std::unique_ptr<DbgEntity>> &getAbstractEntities() {
- if (isDwoUnit() && !DD->shareAcrossDWOCUs())
- return AbstractEntities;
- return DU->getAbstractEntities();
+ return getDIEs().getAbstractEntities();
}
auto &getFinalizedAbstractSubprograms() {
- if (isDwoUnit() && !DD->shareAcrossDWOCUs())
- return FinalizedAbstractSubprograms;
- return DU->getFinalizedAbstractSubprograms();
+ return getDIEs().getFinalizedAbstractSubprograms();
}
void finishNonUnitTypeDIE(DIE& D, const DICompositeType *CTy) override;
@@ -327,7 +319,7 @@ class DwarfCompileUnit final : public DwarfUnit {
DIE *getOrCreateImportedEntityDIE(const DIImportedEntity *IE);
DIE *constructImportedEntityDIE(const DIImportedEntity *IE);
- void finishSubprogramDefinition(const DISubprogram *SP);
+ void finishSubprogramDefinition(const DISubprogram *SP, const Function *F);
void finishEntityDefinition(const DbgEntity *Entity);
void attachLexicalScopesAbstractOrigins();
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index d751a7f9f01ef..865e7b7d6d376 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -502,7 +502,8 @@ void DwarfDebug::addSubprogramNames(
// well into the name table. Only do that if we are going to actually emit
// that name.
if (LinkageName != "" && SP->getName() != LinkageName &&
- (useAllLinkageNames() || InfoHolder.getAbstractScopeDIEs().lookup(SP)))
+ (useAllLinkageNames() ||
+ InfoHolder.getDIEs().getLocalScopes().getAbstractDIEs().lookup(SP)))
addAccelName(Unit, NameTableKind, LinkageName, Die);
// If this is an Objective-C selector name add it to the ObjC accelerator
@@ -1263,11 +1264,13 @@ void DwarfDebug::finishEntityDefinitions() {
}
void DwarfDebug::finishSubprogramDefinitions() {
- for (const DISubprogram *SP : ProcessedSPNodes) {
+ for (auto SPF : ProcessedSPNodes) {
+ const DISubprogram *SP = SPF.first;
assert(SP->getUnit()->getEmissionKind() != DICompileUnit::NoDebug);
- forBothCUs(
- getOrCreateDwarfCompileUnit(SP->getUnit()),
- [&](DwarfCompileUnit &CU) { CU.finishSubprogramDefinition(SP); });
+ forBothCUs(getOrCreateDwarfCompileUnit(SP->getUnit()),
+ [&](DwarfCompileUnit &CU) {
+ CU.finishSubprogramDefinition(SP, SPF.second);
+ });
}
}
@@ -2747,7 +2750,7 @@ void DwarfDebug::endFunctionImpl(const MachineFunction *MF) {
// is still needed as we need its source location.
if (!TheCU.getCUNode()->getDebugInfoForProfiling() &&
TheCU.getCUNode()->getEmissionKind() == DICompileUnit::LineTablesOnly &&
- LScopes.getAbstractScopesList().empty() && !IsDarwin) {
+ !LScopes.currentFunctionHasInlinedScopes() && !IsDarwin) {
for (const auto &R : Asm->MBBSectionRanges)
addArangeLabel(SymbolCU(&TheCU, R.second.BeginLabel));
@@ -2784,11 +2787,11 @@ void DwarfDebug::endFunctionImpl(const MachineFunction *MF) {
constructAbstractSubprogramScopeDIE(TheCU, AScope);
}
- ProcessedSPNodes.insert(SP);
+ ProcessedSPNodes.insert(std::make_pair(SP, &F));
DIE &ScopeDIE =
TheCU.constructSubprogramScopeDIE(SP, F, FnScope, FunctionLineTableLabel);
if (auto *SkelCU = TheCU.getSkeleton())
- if (!LScopes.getAbstractScopesList().empty() &&
+ if (LScopes.currentFunctionHasInlinedScopes() &&
TheCU.getCUNode()->getSplitDebugInlining())
SkelCU->constructSubprogramScopeDIE(SP, F, FnScope,
FunctionLineTableLabel);
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
index 1a1b28a6fc035..42ac225e2d17e 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
@@ -373,7 +373,8 @@ class DwarfDebug : public DebugHandlerBase {
/// This is a collection of subprogram MDNodes that are processed to
/// create DIEs.
- SmallSetVector<const DISubprogram *, 16> ProcessedSPNodes;
+ SmallSetVector<std::pair<const DISubprogram *, const Function *>, 16>
+ ProcessedSPNodes;
/// Map function-local imported entities to their parent local scope
/// (either DILexicalBlock or DISubprogram) for a processed function
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h b/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h
index ef1524d875c84..94d4e5f0b7f05 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h
@@ -15,9 +15,12 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/CodeGen/DIE.h"
+#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/Support/Allocator.h"
+#include <functional>
#include <map>
#include <memory>
+#include <optional>
#include <utility>
namespace llvm {
@@ -26,9 +29,6 @@ class AsmPrinter;
class DbgEntity;
class DbgVariable;
class DbgLabel;
-class DINode;
-class DILocalScope;
-class DISubprogram;
class DwarfCompileUnit;
class DwarfUnit;
class LexicalScope;
@@ -53,6 +53,144 @@ struct RangeSpanList {
SmallVector<RangeSpan, 2> Ranges;
};
+/// Tracks abstract and concrete DIEs for debug info entities of a certain type.
+template <typename DINodeT, typename DbgEntityT> class DINodeInfoHolder {
+public:
+ using AbstractMapT = DenseMap<const DINodeT *, DIE *>;
+ using ConcreteMapT =
+ DenseMap<const DINodeT *, SmallDenseMap<const DbgEntityT *, DIE *, 2>>;
+
+private:
+ AbstractMapT AbstractMap;
+ ConcreteMapT ConcreteMap;
+
+public:
+ void insertAbstractDIE(const DINodeT *N, DIE *D) {
+ auto [_, Inserted] = AbstractMap.try_emplace(N, D);
+ assert(Inserted && "Duplicate abstract DIE for debug info node");
+ }
+
+ void insertConcreteDIE(const DINodeT *N, const DbgEntityT *E, DIE *D) {
+ auto [_, Inserted] = ConcreteMap[N].try_emplace(E, D);
+ assert(Inserted && "Duplicate concrete DIE for debug info node");
+ }
+
+ void insertDIE(const DINodeT *N, const DbgEntityT *E, DIE *D, bool Abstract) {
+ if (Abstract)
+ insertAbstractDIE(N, D);
+ else
+ insertConcreteDIE(N, E, D);
+ }
+
+ DIE *getAbstractDIE(const DINodeT *N) const { return AbstractMap.lookup(N); }
+
+ std::optional<
+ std::reference_wrapper<const typename ConcreteMapT::mapped_type>>
+ getConcreteDIEs(const DINodeT *N) const {
+ if (auto I = ConcreteMap.find(N); I != ConcreteMap.end())
+ return std::make_optional(std::ref(I->second));
+ return std::nullopt;
+ }
+
+ DIE *getConcreteDIE(const DINodeT *N, const DbgEntityT *E) const {
+ if (auto I = getConcreteDIEs(N))
+ return I->get().lookup(E);
+ return nullptr;
+ }
+
+ DIE *getAnyConcreteDIE(const DINodeT *N) const {
+ if (auto I = getConcreteDIEs(N))
+ return I->get().empty() ? nullptr : I->get().begin()->second;
+ return nullptr;
+ }
+
+ /// Returns abstract DIE for the entity.
+ /// If no abstract DIE was created, returns any concrete DIE for the entity.
+ DIE *getDIE(const DINodeT *N) const {
+ if (DIE *D = getAbstractDIE(N))
+ return D;
+
+ return getAnyConcreteDIE(N);
+ }
+
+ AbstractMapT &getAbstractDIEs() { return AbstractMap; }
+};
+
+/// Tracks DIEs for debug info entites.
+/// These DIEs can be shared across CUs, that is why we keep the map here
+/// instead of in DwarfCompileUnit.
+class DwarfInfoHolder {
+public:
+ using LocalScopeHolderT = DINodeInfoHolder<DILocalScope, Function>;
+ using AbstractScopeMapT = LocalScopeHolderT::AbstractMapT;
+
+private:
+ /// DIEs of local DbgVariables.
+ DINodeInfoHolder<DILocalVariable, DbgVariable> LVHolder;
+ /// DIEs of labels.
+ DINodeInfoHolder<DILabel, DbgLabel> LabelHolder;
+ DenseMap<const DINode *, std::unique_ptr<DbgEntity>> AbstractEntities;
+ /// DIEs of abstract local scopes and concrete non-inlined subprograms.
+ /// Inlined subprograms and concrete lexical blocks are not stored here.
+ LocalScopeHolderT LSHolder;
+ /// Keeps track of abstract subprograms to populate them only once.
+ // FIXME: merge creation and population of abstract scopes.
+ SmallPtrSet<const DISubprogram *, 8> FinalizedAbstractSubprograms;
+
+ /// Other DINodes with the corresponding DIEs.
+ DenseMap<const DINode *, DIE *> MDNodeToDieMap;
+
+public:
+ void insertDIE(const DINode *N, DIE *Die) {
+ assert((!isa<DILabel>(N) && !isa<DILocalVariable>(N) &&
+ !isa<DILocalScope>(N)) &&
+ "Use getLabels().insertDIE() for labels or getLVs().insertDIE() for "
+ "local variables, or getSubprogram().insertDIE() for subprograms.");
+ auto [_, Inserted] = MDNodeToDieMap.try_emplace(N, Die);
+ assert((Inserted || isa<DIType>(N)) &&
+ "DIE for this DINode has already been added");
+ }
+
+ void insertDIE(DIE *D) { MDNodeToDieMap.try_emplace(nullptr, D); }
+
+ DIE *getDIE(const DINode *N) const {
+ DIE *D = MDNodeToDieMap.lookup(N);
+ assert((!D || (!isa<DILabel>(N) && !isa<DILocalVariable>(N) &&
+ !isa<DILocalScope>(N))) &&
+ "Use getLabels().getDIE() for labels or getLVs().getDIE() for "
+ "local variables, or getLocalScopes().getDIE() for local scopes.");
+ return D;
+ }
+
+ auto &getLVs() { return LVHolder; }
+ auto &getLVs() const { return LVHolder; }
+
+ auto &getLabels() { return LabelHolder; }
+ auto &getLabels() const { return LabelHolder; }
+
+ auto &getLocalScopes() { return LSHolder; }
+ auto &getLocalScopes() const { return LSHolder; }
+
+ /// For a global variable, returns DIE of the variable.
+ ///
+ /// For a local variable, returns abstract DIE of the variable.
+ /// If no abstract DIE was created, returns any concrete DIE of the variable.
+ DIE *getVariableDIE(const DIVariable *V) const {
+ if (auto *LV = dyn_cast<DILocalVariable>(V))
+ if (DIE *D = getLVs().getDIE(LV))
+ return D;
+ return getDIE(V);
+ }
+
+ DenseMap<const DINode *, std::unique_ptr<DbgEntity>> &getAbstractEntities() {
+ return...
[truncated]
|
`DwarfCompileUnit::constructVariableDIE()` and `constructLabelDIE()` are meant for constructing both abstract and concrete DIEs of a DbgEntity. They use `DwarfUnit::insertDIE()` to store a freshly-created DIE. However, `insertDIE()`/`DwarfUnit::DITypeNodeToDieMap` store only single DIE per DINode. If `insertDIE()` is called several times for the same instance of DINode, only first DIE is saved in `DwarfUnit::DITypeNodeToDieMap`, as follows from `DenseMap::insert()` specification. It means, depending on what is called first, `DwarfCompileUnit::constructVariableDIE(LV, /* Abstract */ true)` or `DwarfCompileUnit::constructVariableDIE(LV, /* Abstract */ false)`, `DwarfUnit::DITypeNodeToDieMap` stores either abstract or concrete DIE of a node. This behavior suggests an obscure API of DwarfCompileUnit, as it depends on function call order and makes it unclear what `DwarfUnit::DITypeNodeToDieMap` is meant to store. To address that, DwarfInfoHolder class is introduced, which stores DIEs for DILocalVariables and DILabels separately from DIEs for other DINodes (as DILocalVariables and DILabels may have concrete and abstract DIEs), and allows explicit access to abstract/concrete DIEs of a debug info entity. Also, DwarfFile and DwarfUnit have a tiny duplicate code piece. AbstractEntities, AbstractLocalScopeDIEs and FinalizedAbstractSubprograms tracking were moved to DwarfInfoHolder, as the corresponding entities may be shared across CUs. DwarfInfoHolder may later be used for tracking DIEs of abstract/concrete lexical scopes. Currently, concrete lexical block/subprogram DIEs are distinguished by their DISubprogram/DILocalScope/DILocalScope+inlinedAt in DwarfCompileUnit. As a result, the same DISubprogram can't be attached to two llvm::Functions (https://lists.llvm.org/pipermail/llvm-dev/2020-September/145342.html). Matching DISubprogram/DILocalScope DIEs with their LexicalScopes and letting DwarfUnit members to access abstract scopes may enable linking DISubprogram to several llvm::Functions, and allow the transition from distinct to uniqued DISubprograms proposed here llvm#142166 (comment).
Depends on: * llvm#152680 With this change, DINodeInfoHolder is used to store abstract and concrete out-of-line subprogram DIEs in DwarfInfoHolder. Every definition subprogram DIE is associated with a corresponding llvm::Function (declaration subprograms are associated with nullptr). When a concrete subprogram DIE is queried via `getOrCreateSubprogramDIE`, the corresponding llvm::Function should be provided. If none is provided: * DwarfUnit/DwarfTypeUnit falls back and returns any concrete DIE for the given DISubprogram, * DwarfCompileUnit is expected to return abstract DIE. This is a step to support attachment of a DISubprogram to multiple llvm::Functions (and to establish one-to-one-to-many correspondence between DISubprograms, abstract DIEs and function clones, and, later, to make the backend use uniquied DISubprograms).
…o multiple Functions Depends on: * llvm#152680 * llvm#162852 In llvm#75385 (and the following tries), an attempt was made, to support attaching local types to DILocalScopes, and to store function local types in DISubprogram's `retainedNodes:` field. That patch failed to land due to issues arising during LTO process. If two definition DISubprograms from different compile units represent, essentially, the same source code function, and have common local DICompositeType, and if this DICompositeType is uniqued (due to ODRUniquingDebugTypes feature), the subprograms end up having wrong retainedNodes list/scoping relationship. To tackle this issue, in llvm#142166, it was proposed to force-unique all DISubporgrams even if they don't contain odr-uniqued types (llvm#142166 (comment)). It should establish one-to-one-to-many relationship between DISubprograms, abstract DIEs and function clones (from different CUs, in case of LTO). To implement that, AsmPrinter should support correct emission of debug info for DISubprograms attached to multiple functions. This is the goal of this commit. Here, LexicalScope's function map is changed to multimap between DISubprogram and (possible multiple) functions attached to it. LexicalScope is modified to create an abstract scope for a DISubprogram having multiple lllvm::Function attachments. `DwarfCompileUnit::getOrCreateSubprogramDIE` can recognize the case of DISubprogram attached to multiple Functions, and return abstract DIE when needed. CodeViewDebug is adopted as well. UDTs are ensured to be emmited properly in the cases that are addressed here. Please let me know if more changes to CodeView needed, as I'm not very familiar with the format.
dzhidzhoev
force-pushed
the
debuginfo/rfc-krisb/duplicate-subprograms/base
branch
from
0238e48 to
24f2d67
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on:
In #75385 (and the following tries), an attempt was made, to support attaching local types to DILocalScopes, and to store function local types in DISubprogram's
retainedNodes:field.That patch failed to land due to issues arising during LTO process. If two definition DISubprograms from different compile units represent, essentially, the same source code function, and have common local DICompositeType, and if this DICompositeType is uniqued (due to ODRUniquingDebugTypes feature), the subprograms end up having wrong retainedNodes list/scoping relationship.
To tackle this issue, in #142166, it was proposed to force-unique all DISubporgrams even if they don't contain odr-uniqued types (#142166 (comment)). It should establish one-to-one-to-many relationship between DISubprograms, abstract DIEs and function clones (from different CUs, in case of LTO).
To implement that, AsmPrinter should support correct emission of debug info for DISubprograms attached to multiple functions. This is the goal of this commit.
Here, LexicalScope's function map is replaced with multimap between DISubprogram and (possibly multiple) functions attached to it. LexicalScope is modified to create an abstract scope for a DISubprogram having multiple llvm::Function attachments.
DwarfCompileUnit::getOrCreateSubprogramDIEcan recognize the case of DISubprogram attached to multiple Functions, and return abstract DIE when needed.CodeViewDebug is adopted as well. UDTs are ensured to be emmited properly in the cases that are addressed here. Please let me know if more changes to CodeView needed, as I'm not very familiar with the format.