-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[DebugInfo][DwarfDebug] Separate creation and population of abstract subprogram DIEs #159104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DebugInfo][DwarfDebug] Separate creation and population of abstract subprogram DIEs #159104
Conversation
@llvm/pr-subscribers-debuginfo Author: Vladislav Dzhidzhoev (dzhidzhoev) ChangesWith this change, construction of abstract subprogram DIEs is split in two stages/functions: LexicalScopes class now stores mapping from DISubprograms to their corresponding llvm::Function's. It is supposed to be built before processing of each function (so, now it has a method for "module initialization" alongside the method for "function initialization"). It is used by DwarfCompileUnit to determine whether a DISubprogram needs an abstract DIE before DwarfDebug::beginFunction is invoked. DwarfCompileUnit::getOrCreateSubprogramDIE method is added, which can create an abstract or a concrete DIE for a subprogram. It accepts llvm::Function* argument to determine whether a concrete DIE must be created. This is a temporary fix for #29985. Ideally, it will be fixed by moving global variables and types emission to DwarfDebug::endModule (https://reviews.llvm.org/D144007, https://reviews.llvm.org/D144005). However, I'm not sure if these patches can be merged before #75385. The last attempt to make it mergeable was #142166. @dwblaikie came up with an idea of making DISubprograms unique in LLVM IR. To implement that, I've tried making a patch that allows DwarfDebug to handle multiple llvm::Functions referring to the same DISubprogram, and it also needs parts of the functionality of this patch. Some changes made by @ellishg in #90523 were taken for this commit. Patch is 45.63 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/159104.diff 20 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/DebugHandlerBase.h b/llvm/include/llvm/CodeGen/DebugHandlerBase.h
index 2849497f9a43e..fee4bb116bb87 100644
--- a/llvm/include/llvm/CodeGen/DebugHandlerBase.h
+++ b/llvm/include/llvm/CodeGen/DebugHandlerBase.h
@@ -144,6 +144,8 @@ class DebugHandlerBase : public AsmPrinterHandler {
static bool isUnsignedDIType(const DIType *Ty);
const InstructionOrdering &getInstOrdering() const { return InstOrdering; }
+
+ const LexicalScopes &getLexicalScopes() const { return LScopes; }
};
} // namespace llvm
diff --git a/llvm/include/llvm/CodeGen/LexicalScopes.h b/llvm/include/llvm/CodeGen/LexicalScopes.h
index 777a0035a2c59..1f70b3f162d41 100644
--- a/llvm/include/llvm/CodeGen/LexicalScopes.h
+++ b/llvm/include/llvm/CodeGen/LexicalScopes.h
@@ -143,12 +143,19 @@ class LexicalScopes {
public:
LexicalScopes() = default;
- /// initialize - Scan machine function and constuct lexical scope nest, resets
- /// the instance if necessary.
- LLVM_ABI void initialize(const MachineFunction &);
+ /// initialize - Scan module to build subprogram-to-function map.
+ LLVM_ABI void initialize(const Module &);
- /// releaseMemory - release memory.
- LLVM_ABI void reset();
+ /// scanFunction - Scan machine function and constuct lexical scope nest,
+ /// resets the instance if necessary.
+ LLVM_ABI void scanFunction(const MachineFunction &);
+
+ /// resetModule - Reset the instance so that it's prepared for another module.
+ LLVM_ABI void resetModule();
+
+ /// functionReset - Reset the instance so that it's prepared for another
+ /// function.
+ LLVM_ABI void resetFunction();
/// empty - Return true if there is any lexical scope information available.
bool empty() { return CurrentFnLexicalScope == nullptr; }
@@ -199,6 +206,12 @@ class LexicalScopes {
/// getOrCreateAbstractScope - Find or create an abstract lexical scope.
LLVM_ABI LexicalScope *getOrCreateAbstractScope(const DILocalScope *Scope);
+ /// getFunction - Get function to which the given subprogram is attached, if
+ /// exists.
+ const Function *getFunction(const DISubprogram *SP) const {
+ return FunctionMap.lookup(SP);
+ }
+
private:
/// getOrCreateLexicalScope - Find lexical scope for the given Scope/IA. If
/// not available then create new lexical scope.
@@ -228,6 +241,9 @@ class LexicalScopes {
const MachineFunction *MF = nullptr;
+ /// FunctionMap - Mapping between DISubprograms and IR functions.
+ DenseMap<const DISubprogram *, const Function *> FunctionMap;
+
/// LexicalScopeMap - Tracks the scopes in the current function.
// Use an unordered_map to ensure value pointer validity over insertion.
std::unordered_map<const DILocalScope *, LexicalScope> LexicalScopeMap;
diff --git a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
index 0f3ff985974ce..7cd5f586a7fa3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
@@ -103,8 +103,12 @@ DebugHandlerBase::DebugHandlerBase(AsmPrinter *A) : Asm(A), MMI(Asm->MMI) {}
DebugHandlerBase::~DebugHandlerBase() = default;
void DebugHandlerBase::beginModule(Module *M) {
- if (M->debug_compile_units().empty())
+ if (M->debug_compile_units().empty()) {
Asm = nullptr;
+ return;
+ }
+
+ LScopes.initialize(*M);
}
// Each LexicalScope has first instruction and last instruction to mark
@@ -269,7 +273,7 @@ void DebugHandlerBase::beginFunction(const MachineFunction *MF) {
// Grab the lexical scopes for the function, if we don't have any of those
// then we're not going to be able to do anything.
- LScopes.initialize(*MF);
+ LScopes.scanFunction(*MF);
if (LScopes.empty()) {
beginFunctionImpl(MF);
return;
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index 67f526fe91464..b12d8b60e0bd3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -537,8 +537,9 @@ void DwarfCompileUnit::addWasmRelocBaseGlobal(DIELoc *Loc, StringRef GlobalName,
// and DW_AT_high_pc attributes. If there are global variables in this
// scope then create and insert DIEs for these variables.
DIE &DwarfCompileUnit::updateSubprogramScopeDIE(const DISubprogram *SP,
+ const Function &F,
MCSymbol *LineTableSym) {
- DIE *SPDie = getOrCreateSubprogramDIE(SP, includeMinimalInlineScopes());
+ DIE *SPDie = getOrCreateSubprogramDIE(SP, &F, includeMinimalInlineScopes());
SmallVector<RangeSpan, 2> BB_List;
// If basic block sections are on, ranges for each basic block section has
// to be emitted separately.
@@ -1122,9 +1123,10 @@ sortLocalVars(SmallVectorImpl<DbgVariable *> &Input) {
}
DIE &DwarfCompileUnit::constructSubprogramScopeDIE(const DISubprogram *Sub,
+ const Function &F,
LexicalScope *Scope,
MCSymbol *LineTableSym) {
- DIE &ScopeDIE = updateSubprogramScopeDIE(Sub, LineTableSym);
+ DIE &ScopeDIE = updateSubprogramScopeDIE(Sub, F, LineTableSym);
if (Scope) {
assert(!Scope->getInlinedAt());
@@ -1198,31 +1200,12 @@ DIE *DwarfCompileUnit::createAndAddScopeChildren(LexicalScope *Scope,
return ObjectPointer;
}
-void DwarfCompileUnit::constructAbstractSubprogramScopeDIE(
- LexicalScope *Scope) {
- auto *SP = cast<DISubprogram>(Scope->getScopeNode());
- if (getAbstractScopeDIEs().count(SP))
- return;
+DIE &DwarfCompileUnit::getOrCreateAbstractSubprogramDIE(
+ const DISubprogram *SP) {
+ if (auto *AbsDef = getAbstractScopeDIEs().lookup(SP))
+ return *AbsDef;
- DIE *ContextDIE;
- DwarfCompileUnit *ContextCU = this;
-
- if (includeMinimalInlineScopes())
- ContextDIE = &getUnitDie();
- // Some of this is duplicated from DwarfUnit::getOrCreateSubprogramDIE, with
- // the important distinction that the debug node is not associated with the
- // DIE (since the debug node will be associated with the concrete DIE, if
- // any). It could be refactored to some common utility function.
- else if (auto *SPDecl = SP->getDeclaration()) {
- ContextDIE = &getUnitDie();
- getOrCreateSubprogramDIE(SPDecl);
- } else {
- ContextDIE = getOrCreateContextDIE(SP->getScope());
- // The scope may be shared with a subprogram that has already been
- // constructed in another CU, in which case we need to construct this
- // subprogram in the same CU.
- ContextCU = DD->lookupCU(ContextDIE->getUnitDie());
- }
+ auto [ContextDIE, ContextCU] = getOrCreateAbstractSubprogramContextDIE(SP);
// Passing null as the associated node because the abstract definition
// shouldn't be found by lookup.
@@ -1237,6 +1220,42 @@ void DwarfCompileUnit::constructAbstractSubprogramScopeDIE(
DD->getDwarfVersion() <= 4 ? std::optional<dwarf::Form>()
: dwarf::DW_FORM_implicit_const,
dwarf::DW_INL_inlined);
+
+ return AbsDef;
+}
+
+std::pair<DIE *, DwarfCompileUnit *>
+DwarfCompileUnit::getOrCreateAbstractSubprogramContextDIE(
+ const DISubprogram *SP) {
+ bool Minimal = includeMinimalInlineScopes();
+ bool IgnoreScope = shouldPlaceInUnitDIE(SP, Minimal);
+ DIE *ContextDIE = getOrCreateSubprogramContextDIE(SP, IgnoreScope);
+
+ if (auto *SPDecl = SP->getDeclaration())
+ if (!Minimal)
+ getOrCreateSubprogramDIE(SPDecl, nullptr);
+
+ // The scope may be shared with a subprogram that has already been
+ // constructed in another CU, in which case we need to construct this
+ // subprogram in the same CU.
+ auto *ContextCU = IgnoreScope ? this : DD->lookupCU(ContextDIE->getUnitDie());
+
+ return std::make_pair(ContextDIE, ContextCU);
+}
+
+void DwarfCompileUnit::constructAbstractSubprogramScopeDIE(
+ LexicalScope *Scope) {
+ auto *SP = cast<DISubprogram>(Scope->getScopeNode());
+
+ if (getFinalizedAbstractSubprograms().contains(SP)) {
+ return;
+ }
+
+ DIE &AbsDef = getOrCreateAbstractSubprogramDIE(SP);
+ auto [ContextDIE, ContextCU] = getOrCreateAbstractSubprogramContextDIE(SP);
+
+ getFinalizedAbstractSubprograms().insert(SP);
+
if (DIE *ObjectPointer = ContextCU->createAndAddScopeChildren(Scope, AbsDef))
ContextCU->addDIEEntry(AbsDef, dwarf::DW_AT_object_pointer, *ObjectPointer);
}
@@ -1293,9 +1312,9 @@ DwarfCompileUnit::getDwarf5OrGNULocationAtom(dwarf::LocationAtom Loc) const {
}
DIE &DwarfCompileUnit::constructCallSiteEntryDIE(
- DIE &ScopeDIE, const DISubprogram *CalleeSP, bool IsTail,
- const MCSymbol *PCAddr, const MCSymbol *CallAddr, unsigned CallReg,
- DIType *AllocSiteTy) {
+ DIE &ScopeDIE, const DISubprogram *CalleeSP, const Function *CalleeF,
+ bool IsTail, const MCSymbol *PCAddr, const MCSymbol *CallAddr,
+ unsigned CallReg, DIType *AllocSiteTy) {
// Insert a call site entry DIE within ScopeDIE.
DIE &CallSiteDIE = createAndAddDIE(getDwarf5OrGNUTag(dwarf::DW_TAG_call_site),
ScopeDIE, nullptr);
@@ -1305,7 +1324,7 @@ DIE &DwarfCompileUnit::constructCallSiteEntryDIE(
addAddress(CallSiteDIE, getDwarf5OrGNUAttr(dwarf::DW_AT_call_target),
MachineLocation(CallReg));
} else if (CalleeSP) {
- DIE *CalleeDIE = getOrCreateSubprogramDIE(CalleeSP);
+ DIE *CalleeDIE = getOrCreateSubprogramDIE(CalleeSP, CalleeF);
assert(CalleeDIE && "Could not create DIE for call site entry origin");
if (AddLinkageNamesToDeclCallOriginsForTuning(DD) &&
!CalleeSP->isDefinition() &&
@@ -1396,7 +1415,7 @@ DIE *DwarfCompileUnit::constructImportedEntityDIE(
if (auto *AbsSPDie = getAbstractScopeDIEs().lookup(SP))
EntityDie = AbsSPDie;
else
- EntityDie = getOrCreateSubprogramDIE(SP);
+ EntityDie = getOrCreateSubprogramDIE(SP, nullptr);
} else if (auto *T = dyn_cast<DIType>(Entity))
EntityDie = getOrCreateTypeDIE(T);
else if (auto *GV = dyn_cast<DIGlobalVariable>(Entity))
@@ -1805,3 +1824,16 @@ DIE *DwarfCompileUnit::getOrCreateContextDIE(const DIScope *Context) {
}
return DwarfUnit::getOrCreateContextDIE(Context);
}
+
+DIE *DwarfCompileUnit::getOrCreateSubprogramDIE(const DISubprogram *SP,
+ const Function *F,
+ bool Minimal) {
+ if (!F && SP->isDefinition()) {
+ F = DD->getLexicalScopes().getFunction(SP);
+
+ if (!F)
+ return &getCU().getOrCreateAbstractSubprogramDIE(SP);
+ }
+
+ return DwarfUnit::getOrCreateSubprogramDIE(SP, F, Minimal);
+}
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
index c2f6ca0913818..219df679b2c91 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
@@ -81,6 +81,7 @@ class DwarfCompileUnit final : public DwarfUnit {
// 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.
@@ -137,12 +138,24 @@ class DwarfCompileUnit final : public DwarfUnit {
return DU->getAbstractEntities();
}
+ auto &getFinalizedAbstractSubprograms() {
+ if (isDwoUnit() && !DD->shareAcrossDWOCUs())
+ return FinalizedAbstractSubprograms;
+ return DU->getFinalizedAbstractSubprograms();
+ }
+
void finishNonUnitTypeDIE(DIE& D, const DICompositeType *CTy) override;
/// Add info for Wasm-global-based relocation.
void addWasmRelocBaseGlobal(DIELoc *Loc, StringRef GlobalName,
uint64_t GlobalIndex);
+ /// Create context DIE for abstract subprogram.
+ /// \returns The context DIE and the compile unit where abstract
+ /// DIE should be constructed.
+ std::pair<DIE *, DwarfCompileUnit *>
+ getOrCreateAbstractSubprogramContextDIE(const DISubprogram *SP);
+
public:
DwarfCompileUnit(unsigned UID, const DICompileUnit *Node, AsmPrinter *A,
DwarfDebug *DW, DwarfFile *DWU,
@@ -216,7 +229,8 @@ class DwarfCompileUnit final : public DwarfUnit {
/// DW_AT_low_pc, DW_AT_high_pc and DW_AT_LLVM_stmt_sequence attributes.
/// If there are global variables in this scope then create and insert DIEs
/// for these variables.
- DIE &updateSubprogramScopeDIE(const DISubprogram *SP, MCSymbol *LineTableSym);
+ DIE &updateSubprogramScopeDIE(const DISubprogram *SP, const Function &F,
+ MCSymbol *LineTableSym);
void constructScopeDIE(LexicalScope *Scope, DIE &ParentScopeDIE);
@@ -259,12 +273,18 @@ class DwarfCompileUnit final : public DwarfUnit {
/// This instance of 'getOrCreateContextDIE()' can handle DILocalScope.
DIE *getOrCreateContextDIE(const DIScope *Ty) override;
+ DIE *getOrCreateSubprogramDIE(const DISubprogram *SP, const Function *F,
+ bool Minimal = false) override;
+
/// Construct a DIE for this subprogram scope.
- DIE &constructSubprogramScopeDIE(const DISubprogram *Sub, LexicalScope *Scope,
- MCSymbol *LineTableSym);
+ DIE &constructSubprogramScopeDIE(const DISubprogram *Sub, const Function &F,
+ LexicalScope *Scope, MCSymbol *LineTableSym);
DIE *createAndAddScopeChildren(LexicalScope *Scope, DIE &ScopeDIE);
+ /// Create an abstract subprogram DIE, that should later be populated
+ /// by \ref constructAbstractSubprogramScopeDIE.
+ DIE &getOrCreateAbstractSubprogramDIE(const DISubprogram *SP);
void constructAbstractSubprogramScopeDIE(LexicalScope *Scope);
/// Whether to use the GNU analog for a DWARF5 tag, attribute, or location
@@ -281,14 +301,15 @@ class DwarfCompileUnit final : public DwarfUnit {
dwarf::LocationAtom getDwarf5OrGNULocationAtom(dwarf::LocationAtom Loc) const;
/// Construct a call site entry DIE describing a call within \p Scope to a
- /// callee described by \p CalleeSP.
+ /// callee described by \p CalleeSP and \p CalleeF.
/// \p IsTail specifies whether the call is a tail call.
/// \p PCAddr points to the PC value after the call instruction.
/// \p CallAddr points to the PC value at the call instruction (or is null).
/// \p CallReg is a register location for an indirect call. For direct calls
/// the \p CallReg is set to 0.
DIE &constructCallSiteEntryDIE(DIE &ScopeDIE, const DISubprogram *CalleeSP,
- bool IsTail, const MCSymbol *PCAddr,
+ const Function *CalleeF, bool IsTail,
+ const MCSymbol *PCAddr,
const MCSymbol *CallAddr, unsigned CallReg,
DIType *AllocSiteTy);
/// Construct call site parameter DIEs for the \p CallSiteDIE. The \p Params
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 2090157a1a91c..25e291c53ea6a 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -1001,8 +1001,9 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
->getName(CallReg)))
<< (IsTail ? " [IsTail]" : "") << "\n");
- DIE &CallSiteDIE = CU.constructCallSiteEntryDIE(
- ScopeDIE, CalleeSP, IsTail, PCAddr, CallAddr, CallReg, AllocSiteTy);
+ DIE &CallSiteDIE =
+ CU.constructCallSiteEntryDIE(ScopeDIE, CalleeSP, CalleeDecl, IsTail,
+ PCAddr, CallAddr, CallReg, AllocSiteTy);
// Optionally emit call-site-param debug info.
if (emitDebugEntryValues()) {
@@ -2711,7 +2712,8 @@ void DwarfDebug::skippedNonDebugFunction() {
// Gather and emit post-function debug information.
void DwarfDebug::endFunctionImpl(const MachineFunction *MF) {
- const DISubprogram *SP = MF->getFunction().getSubprogram();
+ const Function &F = MF->getFunction();
+ const DISubprogram *SP = F.getSubprogram();
assert(CurFn == MF &&
"endFunction should be called with the same function as beginFunction");
@@ -2780,11 +2782,12 @@ void DwarfDebug::endFunctionImpl(const MachineFunction *MF) {
ProcessedSPNodes.insert(SP);
DIE &ScopeDIE =
- TheCU.constructSubprogramScopeDIE(SP, FnScope, FunctionLineTableLabel);
+ TheCU.constructSubprogramScopeDIE(SP, F, FnScope, FunctionLineTableLabel);
if (auto *SkelCU = TheCU.getSkeleton())
if (!LScopes.getAbstractScopesList().empty() &&
TheCU.getCUNode()->getSplitDebugInlining())
- SkelCU->constructSubprogramScopeDIE(SP, FnScope, FunctionLineTableLabel);
+ SkelCU->constructSubprogramScopeDIE(SP, F, FnScope,
+ FunctionLineTableLabel);
FunctionLineTableLabel = nullptr;
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h b/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h
index 0fc2b91ddfa91..4a85b774fa330 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h
@@ -11,6 +11,7 @@
#include "DwarfStringPool.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/CodeGen/DIE.h"
@@ -27,6 +28,7 @@ class DbgVariable;
class DbgLabel;
class DINode;
class DILocalScope;
+class DISubprogram;
class DwarfCompileUnit;
class DwarfUnit;
class LexicalScope;
@@ -94,6 +96,8 @@ class DwarfFile {
// Collection of abstract subprogram DIEs.
DenseMap<const DILocalScope *, DIE *> AbstractLocalScopeDIEs;
DenseMap<const DINode *, std::unique_ptr<DbgEntity>> AbstractEntities;
+ // FIXME: merge creation and population of abstract scopes.
+ SmallPtrSet<const DISubprogram *, 8> FinalizedAbstractSubprograms;
/// Maps MDNodes for type system with the corresponding DIEs. These DIEs can
/// be shared across CUs, that is why we keep the map here instead
@@ -174,6 +178,10 @@ class DwarfFile {
return AbstractEntities;
}
+ auto &getFinalizedAbstractSubprograms() {
+ return FinalizedAbstractSubprograms;
+ }
+
void insertDIE(const MDNode *TypeMD, DIE *Die) {
DITypeNodeToDieMap.insert(std::make_pair(TypeMD, Die));
}
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index d76fd0c010209..62fb5eb011cf2 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -573,7 +573,7 @@ DIE *DwarfUnit::getOrCreateContextDIE(const DIScope *Context) {
if (auto *NS = dyn_cast<DINamespace>(Context))
return getOrCreateNameSpace(NS);
if (auto *SP = dyn_cast<DISubprogram>(Context))
- return getOrCreateSubprogramDIE(SP);
+ return getOrCreateSubprogramDIE(SP, nullptr);
if (auto *M = dyn_cast<DIModule>(Context))
return getOrCreateModule(M);
return getDIE(Context);
@@ -1066,7 +1066,7 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DICompositeType *CTy) {
if (!Element)
continue;
if (auto *SP = dyn_cast<DISubprogram>(Element))
- getOrCreateSubprogramDIE(SP);
+ getOrCreateSubprogramDIE(SP, nullptr);
else if (auto *DDTy = dyn_cast<DIDerivedType>(Element)) {
if (DDTy->getTag() == dwarf::DW_TAG_friend) {
DIE &ElemDie = createAndAddDIE(dwarf::DW_TAG_friend, Buffer);
@@ -1335,22 +1335,21 @@ DIE *DwarfUnit::getOrCreateModule(const DIModule *M) {
return &MDie;
}
-DIE *DwarfUnit::getOrCreateSubprogramDIE(const DISubprogram *SP, bool Minimal) {
+DIE *DwarfUnit::getOrCreateSubprogramDIE(const DISubprogram *SP,
+ const Function *FnHint, bool Minimal) {
// Construct the context before querying for the existence of the DIE in case
// such construction creates the DIE (as is the case for member function
// declarations).
DIE *ContextDIE =
- Minimal ? &getUnitDie() : getOrCreateContextDIE(SP->getScope());
+ getOrCreateSubprogramContextDIE(SP, shouldPlaceInUnitDIE(SP, ...
[truncated]
|
@llvm/pr-subscribers-backend-x86 Author: Vladislav Dzhidzhoev (dzhidzhoev) ChangesWith this change, construction of abstract subprogram DIEs is split in two stages/functions: LexicalScopes class now stores mapping from DISubprograms to their corresponding llvm::Function's. It is supposed to be built before processing of each function (so, now it has a method for "module initialization" alongside the method for "function initialization"). It is used by DwarfCompileUnit to determine whether a DISubprogram needs an abstract DIE before DwarfDebug::beginFunction is invoked. DwarfCompileUnit::getOrCreateSubprogramDIE method is added, which can create an abstract or a concrete DIE for a subprogram. It accepts llvm::Function* argument to determine whether a concrete DIE must be created. This is a temporary fix for #29985. Ideally, it will be fixed by moving global variables and types emission to DwarfDebug::endModule (https://reviews.llvm.org/D144007, https://reviews.llvm.org/D144005). However, I'm not sure if these patches can be merged before #75385. The last attempt to make it mergeable was #142166. @dwblaikie came up with an idea of making DISubprograms unique in LLVM IR. To implement that, I've tried making a patch that allows DwarfDebug to handle multiple llvm::Functions referring to the same DISubprogram, and it also needs parts of the functionality of this patch. Some changes made by @ellishg in #90523 were taken for this commit. Patch is 45.63 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/159104.diff 20 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/DebugHandlerBase.h b/llvm/include/llvm/CodeGen/DebugHandlerBase.h
index 2849497f9a43e..fee4bb116bb87 100644
--- a/llvm/include/llvm/CodeGen/DebugHandlerBase.h
+++ b/llvm/include/llvm/CodeGen/DebugHandlerBase.h
@@ -144,6 +144,8 @@ class DebugHandlerBase : public AsmPrinterHandler {
static bool isUnsignedDIType(const DIType *Ty);
const InstructionOrdering &getInstOrdering() const { return InstOrdering; }
+
+ const LexicalScopes &getLexicalScopes() const { return LScopes; }
};
} // namespace llvm
diff --git a/llvm/include/llvm/CodeGen/LexicalScopes.h b/llvm/include/llvm/CodeGen/LexicalScopes.h
index 777a0035a2c59..1f70b3f162d41 100644
--- a/llvm/include/llvm/CodeGen/LexicalScopes.h
+++ b/llvm/include/llvm/CodeGen/LexicalScopes.h
@@ -143,12 +143,19 @@ class LexicalScopes {
public:
LexicalScopes() = default;
- /// initialize - Scan machine function and constuct lexical scope nest, resets
- /// the instance if necessary.
- LLVM_ABI void initialize(const MachineFunction &);
+ /// initialize - Scan module to build subprogram-to-function map.
+ LLVM_ABI void initialize(const Module &);
- /// releaseMemory - release memory.
- LLVM_ABI void reset();
+ /// scanFunction - Scan machine function and constuct lexical scope nest,
+ /// resets the instance if necessary.
+ LLVM_ABI void scanFunction(const MachineFunction &);
+
+ /// resetModule - Reset the instance so that it's prepared for another module.
+ LLVM_ABI void resetModule();
+
+ /// functionReset - Reset the instance so that it's prepared for another
+ /// function.
+ LLVM_ABI void resetFunction();
/// empty - Return true if there is any lexical scope information available.
bool empty() { return CurrentFnLexicalScope == nullptr; }
@@ -199,6 +206,12 @@ class LexicalScopes {
/// getOrCreateAbstractScope - Find or create an abstract lexical scope.
LLVM_ABI LexicalScope *getOrCreateAbstractScope(const DILocalScope *Scope);
+ /// getFunction - Get function to which the given subprogram is attached, if
+ /// exists.
+ const Function *getFunction(const DISubprogram *SP) const {
+ return FunctionMap.lookup(SP);
+ }
+
private:
/// getOrCreateLexicalScope - Find lexical scope for the given Scope/IA. If
/// not available then create new lexical scope.
@@ -228,6 +241,9 @@ class LexicalScopes {
const MachineFunction *MF = nullptr;
+ /// FunctionMap - Mapping between DISubprograms and IR functions.
+ DenseMap<const DISubprogram *, const Function *> FunctionMap;
+
/// LexicalScopeMap - Tracks the scopes in the current function.
// Use an unordered_map to ensure value pointer validity over insertion.
std::unordered_map<const DILocalScope *, LexicalScope> LexicalScopeMap;
diff --git a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
index 0f3ff985974ce..7cd5f586a7fa3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
@@ -103,8 +103,12 @@ DebugHandlerBase::DebugHandlerBase(AsmPrinter *A) : Asm(A), MMI(Asm->MMI) {}
DebugHandlerBase::~DebugHandlerBase() = default;
void DebugHandlerBase::beginModule(Module *M) {
- if (M->debug_compile_units().empty())
+ if (M->debug_compile_units().empty()) {
Asm = nullptr;
+ return;
+ }
+
+ LScopes.initialize(*M);
}
// Each LexicalScope has first instruction and last instruction to mark
@@ -269,7 +273,7 @@ void DebugHandlerBase::beginFunction(const MachineFunction *MF) {
// Grab the lexical scopes for the function, if we don't have any of those
// then we're not going to be able to do anything.
- LScopes.initialize(*MF);
+ LScopes.scanFunction(*MF);
if (LScopes.empty()) {
beginFunctionImpl(MF);
return;
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index 67f526fe91464..b12d8b60e0bd3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -537,8 +537,9 @@ void DwarfCompileUnit::addWasmRelocBaseGlobal(DIELoc *Loc, StringRef GlobalName,
// and DW_AT_high_pc attributes. If there are global variables in this
// scope then create and insert DIEs for these variables.
DIE &DwarfCompileUnit::updateSubprogramScopeDIE(const DISubprogram *SP,
+ const Function &F,
MCSymbol *LineTableSym) {
- DIE *SPDie = getOrCreateSubprogramDIE(SP, includeMinimalInlineScopes());
+ DIE *SPDie = getOrCreateSubprogramDIE(SP, &F, includeMinimalInlineScopes());
SmallVector<RangeSpan, 2> BB_List;
// If basic block sections are on, ranges for each basic block section has
// to be emitted separately.
@@ -1122,9 +1123,10 @@ sortLocalVars(SmallVectorImpl<DbgVariable *> &Input) {
}
DIE &DwarfCompileUnit::constructSubprogramScopeDIE(const DISubprogram *Sub,
+ const Function &F,
LexicalScope *Scope,
MCSymbol *LineTableSym) {
- DIE &ScopeDIE = updateSubprogramScopeDIE(Sub, LineTableSym);
+ DIE &ScopeDIE = updateSubprogramScopeDIE(Sub, F, LineTableSym);
if (Scope) {
assert(!Scope->getInlinedAt());
@@ -1198,31 +1200,12 @@ DIE *DwarfCompileUnit::createAndAddScopeChildren(LexicalScope *Scope,
return ObjectPointer;
}
-void DwarfCompileUnit::constructAbstractSubprogramScopeDIE(
- LexicalScope *Scope) {
- auto *SP = cast<DISubprogram>(Scope->getScopeNode());
- if (getAbstractScopeDIEs().count(SP))
- return;
+DIE &DwarfCompileUnit::getOrCreateAbstractSubprogramDIE(
+ const DISubprogram *SP) {
+ if (auto *AbsDef = getAbstractScopeDIEs().lookup(SP))
+ return *AbsDef;
- DIE *ContextDIE;
- DwarfCompileUnit *ContextCU = this;
-
- if (includeMinimalInlineScopes())
- ContextDIE = &getUnitDie();
- // Some of this is duplicated from DwarfUnit::getOrCreateSubprogramDIE, with
- // the important distinction that the debug node is not associated with the
- // DIE (since the debug node will be associated with the concrete DIE, if
- // any). It could be refactored to some common utility function.
- else if (auto *SPDecl = SP->getDeclaration()) {
- ContextDIE = &getUnitDie();
- getOrCreateSubprogramDIE(SPDecl);
- } else {
- ContextDIE = getOrCreateContextDIE(SP->getScope());
- // The scope may be shared with a subprogram that has already been
- // constructed in another CU, in which case we need to construct this
- // subprogram in the same CU.
- ContextCU = DD->lookupCU(ContextDIE->getUnitDie());
- }
+ auto [ContextDIE, ContextCU] = getOrCreateAbstractSubprogramContextDIE(SP);
// Passing null as the associated node because the abstract definition
// shouldn't be found by lookup.
@@ -1237,6 +1220,42 @@ void DwarfCompileUnit::constructAbstractSubprogramScopeDIE(
DD->getDwarfVersion() <= 4 ? std::optional<dwarf::Form>()
: dwarf::DW_FORM_implicit_const,
dwarf::DW_INL_inlined);
+
+ return AbsDef;
+}
+
+std::pair<DIE *, DwarfCompileUnit *>
+DwarfCompileUnit::getOrCreateAbstractSubprogramContextDIE(
+ const DISubprogram *SP) {
+ bool Minimal = includeMinimalInlineScopes();
+ bool IgnoreScope = shouldPlaceInUnitDIE(SP, Minimal);
+ DIE *ContextDIE = getOrCreateSubprogramContextDIE(SP, IgnoreScope);
+
+ if (auto *SPDecl = SP->getDeclaration())
+ if (!Minimal)
+ getOrCreateSubprogramDIE(SPDecl, nullptr);
+
+ // The scope may be shared with a subprogram that has already been
+ // constructed in another CU, in which case we need to construct this
+ // subprogram in the same CU.
+ auto *ContextCU = IgnoreScope ? this : DD->lookupCU(ContextDIE->getUnitDie());
+
+ return std::make_pair(ContextDIE, ContextCU);
+}
+
+void DwarfCompileUnit::constructAbstractSubprogramScopeDIE(
+ LexicalScope *Scope) {
+ auto *SP = cast<DISubprogram>(Scope->getScopeNode());
+
+ if (getFinalizedAbstractSubprograms().contains(SP)) {
+ return;
+ }
+
+ DIE &AbsDef = getOrCreateAbstractSubprogramDIE(SP);
+ auto [ContextDIE, ContextCU] = getOrCreateAbstractSubprogramContextDIE(SP);
+
+ getFinalizedAbstractSubprograms().insert(SP);
+
if (DIE *ObjectPointer = ContextCU->createAndAddScopeChildren(Scope, AbsDef))
ContextCU->addDIEEntry(AbsDef, dwarf::DW_AT_object_pointer, *ObjectPointer);
}
@@ -1293,9 +1312,9 @@ DwarfCompileUnit::getDwarf5OrGNULocationAtom(dwarf::LocationAtom Loc) const {
}
DIE &DwarfCompileUnit::constructCallSiteEntryDIE(
- DIE &ScopeDIE, const DISubprogram *CalleeSP, bool IsTail,
- const MCSymbol *PCAddr, const MCSymbol *CallAddr, unsigned CallReg,
- DIType *AllocSiteTy) {
+ DIE &ScopeDIE, const DISubprogram *CalleeSP, const Function *CalleeF,
+ bool IsTail, const MCSymbol *PCAddr, const MCSymbol *CallAddr,
+ unsigned CallReg, DIType *AllocSiteTy) {
// Insert a call site entry DIE within ScopeDIE.
DIE &CallSiteDIE = createAndAddDIE(getDwarf5OrGNUTag(dwarf::DW_TAG_call_site),
ScopeDIE, nullptr);
@@ -1305,7 +1324,7 @@ DIE &DwarfCompileUnit::constructCallSiteEntryDIE(
addAddress(CallSiteDIE, getDwarf5OrGNUAttr(dwarf::DW_AT_call_target),
MachineLocation(CallReg));
} else if (CalleeSP) {
- DIE *CalleeDIE = getOrCreateSubprogramDIE(CalleeSP);
+ DIE *CalleeDIE = getOrCreateSubprogramDIE(CalleeSP, CalleeF);
assert(CalleeDIE && "Could not create DIE for call site entry origin");
if (AddLinkageNamesToDeclCallOriginsForTuning(DD) &&
!CalleeSP->isDefinition() &&
@@ -1396,7 +1415,7 @@ DIE *DwarfCompileUnit::constructImportedEntityDIE(
if (auto *AbsSPDie = getAbstractScopeDIEs().lookup(SP))
EntityDie = AbsSPDie;
else
- EntityDie = getOrCreateSubprogramDIE(SP);
+ EntityDie = getOrCreateSubprogramDIE(SP, nullptr);
} else if (auto *T = dyn_cast<DIType>(Entity))
EntityDie = getOrCreateTypeDIE(T);
else if (auto *GV = dyn_cast<DIGlobalVariable>(Entity))
@@ -1805,3 +1824,16 @@ DIE *DwarfCompileUnit::getOrCreateContextDIE(const DIScope *Context) {
}
return DwarfUnit::getOrCreateContextDIE(Context);
}
+
+DIE *DwarfCompileUnit::getOrCreateSubprogramDIE(const DISubprogram *SP,
+ const Function *F,
+ bool Minimal) {
+ if (!F && SP->isDefinition()) {
+ F = DD->getLexicalScopes().getFunction(SP);
+
+ if (!F)
+ return &getCU().getOrCreateAbstractSubprogramDIE(SP);
+ }
+
+ return DwarfUnit::getOrCreateSubprogramDIE(SP, F, Minimal);
+}
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
index c2f6ca0913818..219df679b2c91 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
@@ -81,6 +81,7 @@ class DwarfCompileUnit final : public DwarfUnit {
// 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.
@@ -137,12 +138,24 @@ class DwarfCompileUnit final : public DwarfUnit {
return DU->getAbstractEntities();
}
+ auto &getFinalizedAbstractSubprograms() {
+ if (isDwoUnit() && !DD->shareAcrossDWOCUs())
+ return FinalizedAbstractSubprograms;
+ return DU->getFinalizedAbstractSubprograms();
+ }
+
void finishNonUnitTypeDIE(DIE& D, const DICompositeType *CTy) override;
/// Add info for Wasm-global-based relocation.
void addWasmRelocBaseGlobal(DIELoc *Loc, StringRef GlobalName,
uint64_t GlobalIndex);
+ /// Create context DIE for abstract subprogram.
+ /// \returns The context DIE and the compile unit where abstract
+ /// DIE should be constructed.
+ std::pair<DIE *, DwarfCompileUnit *>
+ getOrCreateAbstractSubprogramContextDIE(const DISubprogram *SP);
+
public:
DwarfCompileUnit(unsigned UID, const DICompileUnit *Node, AsmPrinter *A,
DwarfDebug *DW, DwarfFile *DWU,
@@ -216,7 +229,8 @@ class DwarfCompileUnit final : public DwarfUnit {
/// DW_AT_low_pc, DW_AT_high_pc and DW_AT_LLVM_stmt_sequence attributes.
/// If there are global variables in this scope then create and insert DIEs
/// for these variables.
- DIE &updateSubprogramScopeDIE(const DISubprogram *SP, MCSymbol *LineTableSym);
+ DIE &updateSubprogramScopeDIE(const DISubprogram *SP, const Function &F,
+ MCSymbol *LineTableSym);
void constructScopeDIE(LexicalScope *Scope, DIE &ParentScopeDIE);
@@ -259,12 +273,18 @@ class DwarfCompileUnit final : public DwarfUnit {
/// This instance of 'getOrCreateContextDIE()' can handle DILocalScope.
DIE *getOrCreateContextDIE(const DIScope *Ty) override;
+ DIE *getOrCreateSubprogramDIE(const DISubprogram *SP, const Function *F,
+ bool Minimal = false) override;
+
/// Construct a DIE for this subprogram scope.
- DIE &constructSubprogramScopeDIE(const DISubprogram *Sub, LexicalScope *Scope,
- MCSymbol *LineTableSym);
+ DIE &constructSubprogramScopeDIE(const DISubprogram *Sub, const Function &F,
+ LexicalScope *Scope, MCSymbol *LineTableSym);
DIE *createAndAddScopeChildren(LexicalScope *Scope, DIE &ScopeDIE);
+ /// Create an abstract subprogram DIE, that should later be populated
+ /// by \ref constructAbstractSubprogramScopeDIE.
+ DIE &getOrCreateAbstractSubprogramDIE(const DISubprogram *SP);
void constructAbstractSubprogramScopeDIE(LexicalScope *Scope);
/// Whether to use the GNU analog for a DWARF5 tag, attribute, or location
@@ -281,14 +301,15 @@ class DwarfCompileUnit final : public DwarfUnit {
dwarf::LocationAtom getDwarf5OrGNULocationAtom(dwarf::LocationAtom Loc) const;
/// Construct a call site entry DIE describing a call within \p Scope to a
- /// callee described by \p CalleeSP.
+ /// callee described by \p CalleeSP and \p CalleeF.
/// \p IsTail specifies whether the call is a tail call.
/// \p PCAddr points to the PC value after the call instruction.
/// \p CallAddr points to the PC value at the call instruction (or is null).
/// \p CallReg is a register location for an indirect call. For direct calls
/// the \p CallReg is set to 0.
DIE &constructCallSiteEntryDIE(DIE &ScopeDIE, const DISubprogram *CalleeSP,
- bool IsTail, const MCSymbol *PCAddr,
+ const Function *CalleeF, bool IsTail,
+ const MCSymbol *PCAddr,
const MCSymbol *CallAddr, unsigned CallReg,
DIType *AllocSiteTy);
/// Construct call site parameter DIEs for the \p CallSiteDIE. The \p Params
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 2090157a1a91c..25e291c53ea6a 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -1001,8 +1001,9 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
->getName(CallReg)))
<< (IsTail ? " [IsTail]" : "") << "\n");
- DIE &CallSiteDIE = CU.constructCallSiteEntryDIE(
- ScopeDIE, CalleeSP, IsTail, PCAddr, CallAddr, CallReg, AllocSiteTy);
+ DIE &CallSiteDIE =
+ CU.constructCallSiteEntryDIE(ScopeDIE, CalleeSP, CalleeDecl, IsTail,
+ PCAddr, CallAddr, CallReg, AllocSiteTy);
// Optionally emit call-site-param debug info.
if (emitDebugEntryValues()) {
@@ -2711,7 +2712,8 @@ void DwarfDebug::skippedNonDebugFunction() {
// Gather and emit post-function debug information.
void DwarfDebug::endFunctionImpl(const MachineFunction *MF) {
- const DISubprogram *SP = MF->getFunction().getSubprogram();
+ const Function &F = MF->getFunction();
+ const DISubprogram *SP = F.getSubprogram();
assert(CurFn == MF &&
"endFunction should be called with the same function as beginFunction");
@@ -2780,11 +2782,12 @@ void DwarfDebug::endFunctionImpl(const MachineFunction *MF) {
ProcessedSPNodes.insert(SP);
DIE &ScopeDIE =
- TheCU.constructSubprogramScopeDIE(SP, FnScope, FunctionLineTableLabel);
+ TheCU.constructSubprogramScopeDIE(SP, F, FnScope, FunctionLineTableLabel);
if (auto *SkelCU = TheCU.getSkeleton())
if (!LScopes.getAbstractScopesList().empty() &&
TheCU.getCUNode()->getSplitDebugInlining())
- SkelCU->constructSubprogramScopeDIE(SP, FnScope, FunctionLineTableLabel);
+ SkelCU->constructSubprogramScopeDIE(SP, F, FnScope,
+ FunctionLineTableLabel);
FunctionLineTableLabel = nullptr;
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h b/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h
index 0fc2b91ddfa91..4a85b774fa330 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfFile.h
@@ -11,6 +11,7 @@
#include "DwarfStringPool.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/CodeGen/DIE.h"
@@ -27,6 +28,7 @@ class DbgVariable;
class DbgLabel;
class DINode;
class DILocalScope;
+class DISubprogram;
class DwarfCompileUnit;
class DwarfUnit;
class LexicalScope;
@@ -94,6 +96,8 @@ class DwarfFile {
// Collection of abstract subprogram DIEs.
DenseMap<const DILocalScope *, DIE *> AbstractLocalScopeDIEs;
DenseMap<const DINode *, std::unique_ptr<DbgEntity>> AbstractEntities;
+ // FIXME: merge creation and population of abstract scopes.
+ SmallPtrSet<const DISubprogram *, 8> FinalizedAbstractSubprograms;
/// Maps MDNodes for type system with the corresponding DIEs. These DIEs can
/// be shared across CUs, that is why we keep the map here instead
@@ -174,6 +178,10 @@ class DwarfFile {
return AbstractEntities;
}
+ auto &getFinalizedAbstractSubprograms() {
+ return FinalizedAbstractSubprograms;
+ }
+
void insertDIE(const MDNode *TypeMD, DIE *Die) {
DITypeNodeToDieMap.insert(std::make_pair(TypeMD, Die));
}
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index d76fd0c010209..62fb5eb011cf2 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -573,7 +573,7 @@ DIE *DwarfUnit::getOrCreateContextDIE(const DIScope *Context) {
if (auto *NS = dyn_cast<DINamespace>(Context))
return getOrCreateNameSpace(NS);
if (auto *SP = dyn_cast<DISubprogram>(Context))
- return getOrCreateSubprogramDIE(SP);
+ return getOrCreateSubprogramDIE(SP, nullptr);
if (auto *M = dyn_cast<DIModule>(Context))
return getOrCreateModule(M);
return getDIE(Context);
@@ -1066,7 +1066,7 @@ void DwarfUnit::constructTypeDIE(DIE &Buffer, const DICompositeType *CTy) {
if (!Element)
continue;
if (auto *SP = dyn_cast<DISubprogram>(Element))
- getOrCreateSubprogramDIE(SP);
+ getOrCreateSubprogramDIE(SP, nullptr);
else if (auto *DDTy = dyn_cast<DIDerivedType>(Element)) {
if (DDTy->getTag() == dwarf::DW_TAG_friend) {
DIE &ElemDie = createAndAddDIE(dwarf::DW_TAG_friend, Buffer);
@@ -1335,22 +1335,21 @@ DIE *DwarfUnit::getOrCreateModule(const DIModule *M) {
return &MDie;
}
-DIE *DwarfUnit::getOrCreateSubprogramDIE(const DISubprogram *SP, bool Minimal) {
+DIE *DwarfUnit::getOrCreateSubprogramDIE(const DISubprogram *SP,
+ const Function *FnHint, bool Minimal) {
// Construct the context before querying for the existence of the DIE in case
// such construction creates the DIE (as is the case for member function
// declarations).
DIE *ContextDIE =
- Minimal ? &getUnitDie() : getOrCreateContextDIE(SP->getScope());
+ getOrCreateSubprogramContextDIE(SP, shouldPlaceInUnitDIE(SP, ...
[truncated]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, if a separate pass over all the functions in the module is what it takes...
I guess I was trying to avoid that in the past, but it certainly creates challenges trying to decide whether to create an abstract subprogram, what to fill it with, etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// functionReset - Reset the instance so that it's prepared for another | |
/// resetFunction - Reset the instance so that it's prepared for another |
Though, actually, the LLVM coding standards state that the function name should be omitted - perhaps partly for this sort of reason (it can't get out of sync if it's not written): https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments
But I see the names are in other doc comments in this file, so it might look out of place for some to be present and some omitted.
Up to you how you want to go there - don't fix all the others in this patch, but idf you wanted to fix them before/after this patch, that'd be OK. Otherwise I don't mind if you prefer to be consistent with existing practice (with the naming correction here) or diverge and omit the name - I don't mind the inconsistency terribly much here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed these comments in #159310.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skip the {} on this single-line block - consistent with other single-line blocks in this function/LLVM more generally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you say more about the purpose of FinalizedAbstractSubprograms
- especially the separation between where this is queried and where it's inserted? (like what bad thing happens if the insertion happened here at/next to the contains?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a function was inlined into multiple other functions and optimized out, constructAbstractSubprogramScopeDIE will be called twice for the function's DISubprogram. This may cause a subprogram abstract DIE to have duplicate children. We want to avoid that.
like what bad thing happens if the insertion happened here at/next to the contains?
I'm not aware of any issues with merging lookup and insertion here, so I proceeded with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may cause a subprogram abstract DIE to have duplicate children.
This is covered by the test llvm/test/DebugInfo/AArch64/populate-abstract-sp-once.ll
. I've added a comment to DwarfFile::FinalizedAbstractSubprograms
definition, and to DwarfCompileUnit.cpp as well.
…subprogram DIEs With this change, construction of abstract subprogram DIEs is split in two stages/functions: creation of DIE (in DwarfCompileUnit::getOrCreateAbstractSubprogramDIE) and its population with children (in DwarfCompileUnit::constructAbstractSubprogramScopeDIE). With that, abstract subprograms can be created/referenced from DwarfDebug::beginModule, which should solve the issue with static local variables DIE creation of inlined functons with optimized-out definitions. It fixes llvm#29985. LexicalScopes class now stores mapping from DISubprograms to their corresponding llvm::Function's. It is supposed to be built before processing of each function (so, now it has a method for "module initialization" alongside the method for "function initialization"). It is used by DwarfCompileUnit to determine whether a DISubprogram needs an abstract DIE before DwarfDebug::beginFunction is invoked. DwarfCompileUnit::getOrCreateSubprogramDIE method is added, which can create an abstract or a concrete DIE for a subprogram. It accepts llvm::Function* argument to determine whether a concrete DIE must be created. This is a temporary fix for llvm#29985. Ideally, it will be fixed by moving global variables and types emission to DwarfDebug::endModule (https://reviews.llvm.org/D144007, https://reviews.llvm.org/D144005). However, I'm not sure if these patches can be merged before llvm#75385. The last attempt to make it mergeable was llvm#142166. @dwblaikie came up with an idea of making DISubprograms unique in LLVM IR. To implement that, I've tried making a patch that allows DwarfDebug to handle multiple llvm::Functions referring to the same DISubprogram, and it also needs parts of the functionality of this patch. Some changes made by @ellishg in llvm#90523 were taken for this commit.
…AbstractSubprogramScopeDIE
c467ea4
to
af63e86
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's give it a go.
Oh, though - can you rephrase the commit message to avoid tagging usernames in it? Usernames tagged in commits create some unfortunate behavior - whenever that commit is merged into other forks, etc, those users get notified. It can be a bit annoying. |
Thank you!
Sure. |
NB, in some LTO builds we're seeing illegal DIE references that appear to come from faulty-cross-CU-references. It's not yet bisected to this patch (in progress) but I'm reasonably confident that it'll be this, as there's a subprogram-then-dicompositetype-then-subprogram in the scope chain when this happens. I'm optimistic that it'll just be a latent bug being exposed rather than something being fundamentally wrong with this patch. |
Thank you! I'm taking look at the issue. |
Probably worth considering reverting this in the interim - I don't think this patch will bit-rot too quickly while the issue's investigated. Though, being a pre-existing issue, perhaps - be nice if anyone can pitch in (maybe/especially Sony folks, since this is particularly negatively impacting you/you might have some more experience with this than the folks working on the static local scoping work in this & other patches). |
…lation of abstract subprogram DIEs" (#160349) Reverts llvm/llvm-project#159104 due to the issues reported in llvm/llvm-project#160197.
…subprogram DIEs (llvm#159104) With this change, construction of abstract subprogram DIEs is split in two stages/functions: creation of DIE (in DwarfCompileUnit::getOrCreateAbstractSubprogramDIE) and its population with children (in DwarfCompileUnit::constructAbstractSubprogramScopeDIE). With that, abstract subprograms can be created/referenced from DwarfDebug::beginModule, which should solve the issue with static local variables DIE creation of inlined functons with optimized-out definitions. It fixes llvm#29985. LexicalScopes class now stores mapping from DISubprograms to their corresponding llvm::Function's. It is supposed to be built before processing of each function (so, now LexicalScopes class has a method for "module initialization" alongside the method for "function initialization"). It is used by DwarfCompileUnit to determine whether a DISubprogram needs an abstract DIE before DwarfDebug::beginFunction is invoked. DwarfCompileUnit::getOrCreateSubprogramDIE method is added, which can create an abstract or a concrete DIE for a subprogram. It accepts llvm::Function* argument to determine whether a concrete DIE must be created. This is a temporary fix for llvm#29985. Ideally, it will be fixed by moving global variables and types emission to DwarfDebug::endModule (https://reviews.llvm.org/D144007, https://reviews.llvm.org/D144005). Some code proposed by Ellis Hoag <[email protected]> in llvm#90523 was taken for this commit.
…subprogram DIEs (llvm#159104) With this change, construction of abstract subprogram DIEs is split in two stages/functions: creation of DIE (in DwarfCompileUnit::getOrCreateAbstractSubprogramDIE) and its population with children (in DwarfCompileUnit::constructAbstractSubprogramScopeDIE). With that, abstract subprograms can be created/referenced from DwarfDebug::beginModule, which should solve the issue with static local variables DIE creation of inlined functons with optimized-out definitions. It fixes llvm#29985. LexicalScopes class now stores mapping from DISubprograms to their corresponding llvm::Function's. It is supposed to be built before processing of each function (so, now LexicalScopes class has a method for "module initialization" alongside the method for "function initialization"). It is used by DwarfCompileUnit to determine whether a DISubprogram needs an abstract DIE before DwarfDebug::beginFunction is invoked. DwarfCompileUnit::getOrCreateSubprogramDIE method is added, which can create an abstract or a concrete DIE for a subprogram. It accepts llvm::Function* argument to determine whether a concrete DIE must be created. This is a temporary fix for llvm#29985. Ideally, it will be fixed by moving global variables and types emission to DwarfDebug::endModule (https://reviews.llvm.org/D144007, https://reviews.llvm.org/D144005). Some code proposed by Ellis Hoag <[email protected]> in llvm#90523 was taken for this commit.
It turns out, the problem was there https://github.com/dzhidzhoev/llvm-project/blob/cb26204b921862e3c8f7759e3939ca8306d471c1/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp#L1835. return &getCU().getOrCreateAbstractSubprogramDIE(SP); When abstract subprogram DIEs are created in DwarfDebug::endFunctionImpl(), DwarfDebug class makes sure that they are created in the CU a DISubprogram belongs to. DwarfCompileUnit::getOrCreateSubprogramDIE() implementation did not expect that if the DIE for the DISubprogram was queried, it might have been a cross-CU reference. Made fix for that #160786. |
…bstract subprogram DIEs" (#160786) This is an attempt to reland #159104 with the fix for #160197. The original patch had the following problem: when an abstract subprogram DIE is constructed from within `DwarfDebug::endFunctionImpl()`, `DwarfDebug::constructAbstractSubprogramScopeDIE()` acknowledges `unit:` field of DISubprogram. But an abstract subprogram DIE constructed from `DwarfDebug::beginModule()` was put in the same compile unit to which global variable referencing the subprogram belonged, regardless of subprogram's `unit:`. This is fixed by adding `DwarfDebug::getOrCreateAbstractSubprogramCU()` used by both`DwarfDebug:: constructAbstractSubprogramScopeDIE()` and `DwarfCompileUnit::getOrCreateSubprogramDIE()` when abstract subprogram is queried during the creation of DIEs for globals in `DwarfDebug::beginModule()`. The fix and the already-reviewed code from #159104 are two separate commits in this PR. ===== The original commit message follows: With this change, construction of abstract subprogram DIEs is split in two stages/functions: creation of DIE (in DwarfCompileUnit::getOrCreateAbstractSubprogramDIE) and its population with children (in DwarfCompileUnit::constructAbstractSubprogramScopeDIE). With that, abstract subprograms can be created/referenced from DwarfDebug::beginModule, which should solve the issue with static local variables DIE creation of inlined functons with optimized-out definitions. It fixes #29985. LexicalScopes class now stores mapping from DISubprograms to their corresponding llvm::Function's. It is supposed to be built before processing of each function (so, now LexicalScopes class has a method for "module initialization" alongside the method for "function initialization"). It is used by DwarfCompileUnit to determine whether a DISubprogram needs an abstract DIE before DwarfDebug::beginFunction is invoked. DwarfCompileUnit::getOrCreateSubprogramDIE method is added, which can create an abstract or a concrete DIE for a subprogram. It accepts llvm::Function* argument to determine whether a concrete DIE must be created. This is a temporary fix for #29985. Ideally, it will be fixed by moving global variables and types emission to DwarfDebug::endModule (https://reviews.llvm.org/D144007, https://reviews.llvm.org/D144005). Some code proposed by Ellis Hoag <[email protected]> in #90523 was taken for this commit.
…lation of abstract subprogram DIEs" (#160786) This is an attempt to reland llvm/llvm-project#159104 with the fix for llvm/llvm-project#160197. The original patch had the following problem: when an abstract subprogram DIE is constructed from within `DwarfDebug::endFunctionImpl()`, `DwarfDebug::constructAbstractSubprogramScopeDIE()` acknowledges `unit:` field of DISubprogram. But an abstract subprogram DIE constructed from `DwarfDebug::beginModule()` was put in the same compile unit to which global variable referencing the subprogram belonged, regardless of subprogram's `unit:`. This is fixed by adding `DwarfDebug::getOrCreateAbstractSubprogramCU()` used by both`DwarfDebug:: constructAbstractSubprogramScopeDIE()` and `DwarfCompileUnit::getOrCreateSubprogramDIE()` when abstract subprogram is queried during the creation of DIEs for globals in `DwarfDebug::beginModule()`. The fix and the already-reviewed code from llvm/llvm-project#159104 are two separate commits in this PR. ===== The original commit message follows: With this change, construction of abstract subprogram DIEs is split in two stages/functions: creation of DIE (in DwarfCompileUnit::getOrCreateAbstractSubprogramDIE) and its population with children (in DwarfCompileUnit::constructAbstractSubprogramScopeDIE). With that, abstract subprograms can be created/referenced from DwarfDebug::beginModule, which should solve the issue with static local variables DIE creation of inlined functons with optimized-out definitions. It fixes llvm/llvm-project#29985. LexicalScopes class now stores mapping from DISubprograms to their corresponding llvm::Function's. It is supposed to be built before processing of each function (so, now LexicalScopes class has a method for "module initialization" alongside the method for "function initialization"). It is used by DwarfCompileUnit to determine whether a DISubprogram needs an abstract DIE before DwarfDebug::beginFunction is invoked. DwarfCompileUnit::getOrCreateSubprogramDIE method is added, which can create an abstract or a concrete DIE for a subprogram. It accepts llvm::Function* argument to determine whether a concrete DIE must be created. This is a temporary fix for llvm/llvm-project#29985. Ideally, it will be fixed by moving global variables and types emission to DwarfDebug::endModule (https://reviews.llvm.org/D144007, https://reviews.llvm.org/D144005). Some code proposed by Ellis Hoag <[email protected]> in llvm/llvm-project#90523 was taken for this commit.
…bstract subprogram DIEs" (llvm#160786) This is an attempt to reland llvm#159104 with the fix for llvm#160197. The original patch had the following problem: when an abstract subprogram DIE is constructed from within `DwarfDebug::endFunctionImpl()`, `DwarfDebug::constructAbstractSubprogramScopeDIE()` acknowledges `unit:` field of DISubprogram. But an abstract subprogram DIE constructed from `DwarfDebug::beginModule()` was put in the same compile unit to which global variable referencing the subprogram belonged, regardless of subprogram's `unit:`. This is fixed by adding `DwarfDebug::getOrCreateAbstractSubprogramCU()` used by both`DwarfDebug:: constructAbstractSubprogramScopeDIE()` and `DwarfCompileUnit::getOrCreateSubprogramDIE()` when abstract subprogram is queried during the creation of DIEs for globals in `DwarfDebug::beginModule()`. The fix and the already-reviewed code from llvm#159104 are two separate commits in this PR. ===== The original commit message follows: With this change, construction of abstract subprogram DIEs is split in two stages/functions: creation of DIE (in DwarfCompileUnit::getOrCreateAbstractSubprogramDIE) and its population with children (in DwarfCompileUnit::constructAbstractSubprogramScopeDIE). With that, abstract subprograms can be created/referenced from DwarfDebug::beginModule, which should solve the issue with static local variables DIE creation of inlined functons with optimized-out definitions. It fixes llvm#29985. LexicalScopes class now stores mapping from DISubprograms to their corresponding llvm::Function's. It is supposed to be built before processing of each function (so, now LexicalScopes class has a method for "module initialization" alongside the method for "function initialization"). It is used by DwarfCompileUnit to determine whether a DISubprogram needs an abstract DIE before DwarfDebug::beginFunction is invoked. DwarfCompileUnit::getOrCreateSubprogramDIE method is added, which can create an abstract or a concrete DIE for a subprogram. It accepts llvm::Function* argument to determine whether a concrete DIE must be created. This is a temporary fix for llvm#29985. Ideally, it will be fixed by moving global variables and types emission to DwarfDebug::endModule (https://reviews.llvm.org/D144007, https://reviews.llvm.org/D144005). Some code proposed by Ellis Hoag <[email protected]> in llvm#90523 was taken for this commit.
With this change, construction of abstract subprogram DIEs is split in two stages/functions:
creation of DIE (in DwarfCompileUnit::getOrCreateAbstractSubprogramDIE) and its population with children (in DwarfCompileUnit::constructAbstractSubprogramScopeDIE).
With that, abstract subprograms can be created/referenced from DwarfDebug::beginModule, which should solve the issue with static local variables DIE creation of inlined functons with optimized-out definitions. It fixes #29985.
LexicalScopes class now stores mapping from DISubprograms to their corresponding llvm::Function's. It is supposed to be built before processing of each function (so, now LexicalScopes class has a method for "module initialization" alongside the method for "function initialization"). It is used by DwarfCompileUnit to determine whether a DISubprogram needs an abstract DIE before DwarfDebug::beginFunction is invoked.
DwarfCompileUnit::getOrCreateSubprogramDIE method is added, which can create an abstract or a concrete DIE for a subprogram. It accepts llvm::Function* argument to determine whether a concrete DIE must be created.
This is a temporary fix for #29985. Ideally, it will be fixed by moving global variables and types emission to DwarfDebug::endModule (https://reviews.llvm.org/D144007, https://reviews.llvm.org/D144005). However, I'm not sure if these patches can be merged before #75385. The last attempt to make it mergeable was #142166. @dwblaikie came up with an idea of making DISubprograms unique in LLVM IR. To implement that, I've tried making a patch that allows DwarfDebug to handle multiple llvm::Functions referring to the same DISubprogram, and it also needs parts of the functionality of this patch.
Some code proposed by @ellishg in #90523 was taken for this commit.