[IR] Remove the DIExpression field from DIGlobalVariable.
This patch implements PR31013 by introducing a
DIGlobalVariableExpression that holds a pair of DIGlobalVariable and
DIExpression.
Currently, DIGlobalVariables holds a DIExpression. This is not the
best way to model this:
(1) The DIGlobalVariable should describe the source level variable,
not how to get to its location.
(2) It makes it unsafe/hard to update the expressions when we call
replaceExpression on the DIGLobalVariable.
(3) It makes it impossible to represent a global variable that is in
more than one location (e.g., a variable with multiple
DW_OP_LLVM_fragment-s). We also moved away from attaching the
DIExpression to DILocalVariable for the same reasons.
<rdar://problem/29250149>
https://llvm.org/bugs/show_bug.cgi?id=31013
Differential Revision: https://reviews.llvm.org/D26769
llvm-svn: 289902
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index 8e17032..3f5ef1a 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -2181,12 +2181,13 @@
}
void CodeViewDebug::emitDebugInfoForGlobals() {
- DenseMap<const DIGlobalVariable *, const GlobalVariable *> GlobalMap;
+ DenseMap<const DIGlobalVariableExpression *, const GlobalVariable *>
+ GlobalMap;
for (const GlobalVariable &GV : MMI->getModule()->globals()) {
- SmallVector<MDNode *, 1> MDs;
- GV.getMetadata(LLVMContext::MD_dbg, MDs);
- for (MDNode *MD : MDs)
- GlobalMap[cast<DIGlobalVariable>(MD)] = &GV;
+ SmallVector<DIGlobalVariableExpression *, 1> GVEs;
+ GV.getDebugInfo(GVEs);
+ for (const auto *GVE : GVEs)
+ GlobalMap[GVE] = &GV;
}
NamedMDNode *CUs = MMI->getModule()->getNamedMetadata("llvm.dbg.cu");
@@ -2198,14 +2199,15 @@
// it if we have at least one global to emit.
switchToDebugSectionForSymbol(nullptr);
MCSymbol *EndLabel = nullptr;
- for (const DIGlobalVariable *G : CU->getGlobalVariables()) {
- if (const auto *GV = GlobalMap.lookup(G))
+ for (const auto *GVE : CU->getGlobalVariables()) {
+ if (const auto *GV = GlobalMap.lookup(GVE))
if (!GV->hasComdat() && !GV->isDeclarationForLinker()) {
if (!EndLabel) {
OS.AddComment("Symbol subsection for globals");
EndLabel = beginCVSubsection(ModuleSubstreamKind::Symbols);
}
- emitDebugInfoForGlobal(G, GV, Asm->getSymbol(GV));
+ // FIXME: emitDebugInfoForGlobal() doesn't handle DIExpressions.
+ emitDebugInfoForGlobal(GVE->getVariable(), GV, Asm->getSymbol(GV));
}
}
if (EndLabel)
@@ -2213,15 +2215,16 @@
// Second, emit each global that is in a comdat into its own .debug$S
// section along with its own symbol substream.
- for (const DIGlobalVariable *G : CU->getGlobalVariables()) {
- if (const auto *GV = GlobalMap.lookup(G)) {
+ for (const auto *GVE : CU->getGlobalVariables()) {
+ if (const auto *GV = GlobalMap.lookup(GVE)) {
if (GV->hasComdat()) {
MCSymbol *GVSym = Asm->getSymbol(GV);
OS.AddComment("Symbol subsection for " +
Twine(GlobalValue::getRealLinkageName(GV->getName())));
switchToDebugSectionForSymbol(GVSym);
EndLabel = beginCVSubsection(ModuleSubstreamKind::Symbols);
- emitDebugInfoForGlobal(G, GV, GVSym);
+ // FIXME: emitDebugInfoForGlobal() doesn't handle DIExpressions.
+ emitDebugInfoForGlobal(GVE->getVariable(), GV, GVSym);
endCVSubsection(EndLabel);
}
}
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index 5f9506c..0db623b 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -73,9 +73,8 @@
Asm->OutStreamer->hasRawTextSupport() ? 0 : getUniqueID());
}
-/// getOrCreateGlobalVariableDIE - get or create global variable DIE.
DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(
- const DIGlobalVariable *GV, const GlobalVariable *Global) {
+ const DIGlobalVariable *GV, ArrayRef<GlobalExpr> GlobalExprs) {
// Check for pre-existence.
if (DIE *Die = getDIE(GV))
return Die;
@@ -128,69 +127,76 @@
// Add location.
bool addToAccelTable = false;
+ DIELoc *Loc = nullptr;
+ std::unique_ptr<DIEDwarfExpression> DwarfExpr;
+ bool AllConstant = std::all_of(
+ GlobalExprs.begin(), GlobalExprs.end(),
+ [&](const GlobalExpr GE) {
+ return GE.Expr && GE.Expr->isConstant();
+ });
- DIExpression *Expr = GV->getExpr();
-
- // For compatibility with DWARF 3 and earlier,
- // DW_AT_location(DW_OP_constu, X, DW_OP_stack_value) becomes
- // DW_AT_const_value(X).
- if (Expr && Expr->getNumElements() == 3 &&
- Expr->getElement(0) == dwarf::DW_OP_constu &&
- Expr->getElement(2) == dwarf::DW_OP_stack_value) {
- addConstantValue(*VariableDIE, /*Unsigned=*/true, Expr->getElement(1));
- // We cannot describe the location of dllimport'd variables: the computation
- // of their address requires loads from the IAT.
- } else if (!Global || !Global->hasDLLImportStorageClass()) {
- DIELoc *Loc = new (DIEValueAllocator) DIELoc;
- if (Global) {
- addToAccelTable = true;
- const MCSymbol *Sym = Asm->getSymbol(Global);
- if (Global->isThreadLocal()) {
- if (Asm->TM.Options.EmulatedTLS) {
- // TODO: add debug info for emulated thread local mode.
- } else {
- // FIXME: Make this work with -gsplit-dwarf.
- unsigned PointerSize = Asm->getDataLayout().getPointerSize();
- assert((PointerSize == 4 || PointerSize == 8) &&
- "Add support for other sizes if necessary");
- // Based on GCC's support for TLS:
- if (!DD->useSplitDwarf()) {
- // 1) Start with a constNu of the appropriate pointer size
- addUInt(*Loc, dwarf::DW_FORM_data1, PointerSize == 4
- ? dwarf::DW_OP_const4u
- : dwarf::DW_OP_const8u);
- // 2) containing the (relocated) offset of the TLS variable
- // within the module's TLS block.
- addExpr(*Loc, dwarf::DW_FORM_udata,
- Asm->getObjFileLowering().getDebugThreadLocalSymbol(Sym));
- } else {
- addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_GNU_const_index);
- addUInt(*Loc, dwarf::DW_FORM_udata,
- DD->getAddressPool().getIndex(Sym, /* TLS */ true));
- }
- // 3) followed by an OP to make the debugger do a TLS lookup.
- addUInt(*Loc, dwarf::DW_FORM_data1,
- DD->useGNUTLSOpcode() ? dwarf::DW_OP_GNU_push_tls_address
- : dwarf::DW_OP_form_tls_address);
- }
- } else {
- DD->addArangeLabel(SymbolCU(this, Sym));
- addOpAddress(*Loc, Sym);
+ for (const auto &GE : GlobalExprs) {
+ const GlobalVariable *Global = GE.Var;
+ const DIExpression *Expr = GE.Expr;
+ // For compatibility with DWARF 3 and earlier,
+ // DW_AT_location(DW_OP_constu, X, DW_OP_stack_value) becomes
+ // DW_AT_const_value(X).
+ if (GlobalExprs.size() == 1 && Expr && Expr->isConstant()) {
+ addConstantValue(*VariableDIE, /*Unsigned=*/true, Expr->getElement(1));
+ // We cannot describe the location of dllimport'd variables: the
+ // computation of their address requires loads from the IAT.
+ } else if ((Global && !Global->hasDLLImportStorageClass()) || AllConstant) {
+ if (!Loc) {
+ Loc = new (DIEValueAllocator) DIELoc;
+ DwarfExpr = llvm::make_unique<DIEDwarfExpression>(*Asm, *this, *Loc);
}
-
+ addToAccelTable = true;
+ if (Global) {
+ const MCSymbol *Sym = Asm->getSymbol(Global);
+ if (Global->isThreadLocal()) {
+ if (Asm->TM.Options.EmulatedTLS) {
+ // TODO: add debug info for emulated thread local mode.
+ } else {
+ // FIXME: Make this work with -gsplit-dwarf.
+ unsigned PointerSize = Asm->getDataLayout().getPointerSize();
+ assert((PointerSize == 4 || PointerSize == 8) &&
+ "Add support for other sizes if necessary");
+ // Based on GCC's support for TLS:
+ if (!DD->useSplitDwarf()) {
+ // 1) Start with a constNu of the appropriate pointer size
+ addUInt(*Loc, dwarf::DW_FORM_data1,
+ PointerSize == 4 ? dwarf::DW_OP_const4u
+ : dwarf::DW_OP_const8u);
+ // 2) containing the (relocated) offset of the TLS variable
+ // within the module's TLS block.
+ addExpr(*Loc, dwarf::DW_FORM_udata,
+ Asm->getObjFileLowering().getDebugThreadLocalSymbol(Sym));
+ } else {
+ addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_GNU_const_index);
+ addUInt(*Loc, dwarf::DW_FORM_udata,
+ DD->getAddressPool().getIndex(Sym, /* TLS */ true));
+ }
+ // 3) followed by an OP to make the debugger do a TLS lookup.
+ addUInt(*Loc, dwarf::DW_FORM_data1,
+ DD->useGNUTLSOpcode() ? dwarf::DW_OP_GNU_push_tls_address
+ : dwarf::DW_OP_form_tls_address);
+ }
+ } else {
+ DD->addArangeLabel(SymbolCU(this, Sym));
+ addOpAddress(*Loc, Sym);
+ }
+ }
if (Expr) {
- DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
- DwarfExpr.addFragmentOffset(Expr);
- DwarfExpr.AddExpression(Expr);
- DwarfExpr.finalize();
+ DwarfExpr->addFragmentOffset(Expr);
+ DwarfExpr->AddExpression(Expr);
}
}
-
- addBlock(*VariableDIE, dwarf::DW_AT_location, Loc);
-
- if (DD->useAllLinkageNames())
- addLinkageName(*VariableDIE, GV->getLinkageName());
}
+ if (Loc)
+ addBlock(*VariableDIE, dwarf::DW_AT_location, DwarfExpr->finalize());
+
+ if (DD->useAllLinkageNames())
+ addLinkageName(*VariableDIE, GV->getLinkageName());
if (addToAccelTable) {
DD->addAccelName(GV->getName(), *VariableDIE);
@@ -503,8 +509,7 @@
DwarfExpr.addFragmentOffset(Expr);
DwarfExpr.AddUnsignedConstant(DVInsn->getOperand(0).getImm());
DwarfExpr.AddExpression(Expr);
- DwarfExpr.finalize();
- addBlock(*VariableDie, dwarf::DW_AT_location, Loc);
+ addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
} else
addConstantValue(*VariableDie, DVInsn->getOperand(0), DV.getType());
} else if (DVInsn->getOperand(0).isFPImm())
@@ -534,8 +539,7 @@
DwarfExpr.AddExpression(*Expr);
++Expr;
}
- DwarfExpr.finalize();
- addBlock(*VariableDie, dwarf::DW_AT_location, Loc);
+ addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize());
return VariableDie;
}
@@ -654,7 +658,7 @@
else if (auto *T = dyn_cast<DIType>(Entity))
EntityDie = getOrCreateTypeDIE(T);
else if (auto *GV = dyn_cast<DIGlobalVariable>(Entity))
- EntityDie = getOrCreateGlobalVariableDIE(GV, nullptr);
+ EntityDie = getOrCreateGlobalVariableDIE(GV, {});
else
EntityDie = getDIE(Entity);
assert(EntityDie);
@@ -740,10 +744,8 @@
if (!validReg)
return;
- Expr.finalize();
-
// Now attach the location information to the DIE.
- addBlock(Die, Attribute, Loc);
+ addBlock(Die, Attribute, Expr.finalize());
}
/// Start with the address based on the location provided, and generate the
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
index da20bef..a8025f1 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
@@ -91,9 +91,16 @@
/// Apply the DW_AT_stmt_list from this compile unit to the specified DIE.
void applyStmtList(DIE &D);
- /// getOrCreateGlobalVariableDIE - get or create global variable DIE.
- DIE *getOrCreateGlobalVariableDIE(const DIGlobalVariable *GV,
- const GlobalVariable *Global);
+ /// A pair of GlobalVariable and DIExpression.
+ struct GlobalExpr {
+ const GlobalVariable *Var;
+ const DIExpression *Expr;
+ };
+
+ /// Get or create global variable DIE.
+ DIE *
+ getOrCreateGlobalVariableDIE(const DIGlobalVariable *GV,
+ ArrayRef<GlobalExpr> GlobalExprs);
/// addLabelAddress - Add a dwarf label attribute data and value using
/// either DW_FORM_addr or DW_FORM_GNU_addr_index.
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index f83a340..e6f5590 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -464,6 +464,26 @@
D->addChild(TheCU.constructImportedEntityDIE(N));
}
+/// Sort and unique GVEs by comparing their fragment offset.
+static SmallVectorImpl<DwarfCompileUnit::GlobalExpr> &
+sortGlobalExprs(SmallVectorImpl<DwarfCompileUnit::GlobalExpr> &GVEs) {
+ std::sort(GVEs.begin(), GVEs.end(),
+ [](DwarfCompileUnit::GlobalExpr A, DwarfCompileUnit::GlobalExpr B) {
+ if (A.Expr != B.Expr && A.Expr && B.Expr &&
+ A.Expr->isFragment() && B.Expr->isFragment())
+ return A.Expr->getFragmentOffsetInBits() <
+ B.Expr->getFragmentOffsetInBits();
+ return false;
+ });
+ GVEs.erase(std::unique(GVEs.begin(), GVEs.end(),
+ [](DwarfCompileUnit::GlobalExpr A,
+ DwarfCompileUnit::GlobalExpr B) {
+ return A.Expr == B.Expr;
+ }),
+ GVEs.end());
+ return GVEs;
+}
+
// Emit all Dwarf sections that should come prior to the content. Create
// global DIEs and emit initial debug info sections. This is invoked by
// the target AsmPrinter.
@@ -480,21 +500,30 @@
// Tell MMI whether we have debug info.
MMI->setDebugInfoAvailability(NumDebugCUs > 0);
SingleCU = NumDebugCUs == 1;
-
- DenseMap<DIGlobalVariable *, const GlobalVariable *> GVMap;
+ DenseMap<DIGlobalVariable *, SmallVector<DwarfCompileUnit::GlobalExpr, 1>>
+ GVMap;
for (const GlobalVariable &Global : M->globals()) {
- SmallVector<DIGlobalVariable *, 1> GVs;
+ SmallVector<DIGlobalVariableExpression *, 1> GVs;
Global.getDebugInfo(GVs);
- for (auto &GV : GVs)
- GVMap[GV] = &Global;
+ for (auto *GVE : GVs)
+ GVMap[GVE->getVariable()].push_back({&Global, GVE->getExpression()});
}
for (DICompileUnit *CUNode : M->debug_compile_units()) {
DwarfCompileUnit &CU = constructDwarfCompileUnit(CUNode);
for (auto *IE : CUNode->getImportedEntities())
CU.addImportedEntity(IE);
- for (auto *GV : CUNode->getGlobalVariables())
- CU.getOrCreateGlobalVariableDIE(GV, GVMap.lookup(GV));
+
+ // Global Variables.
+ for (auto *GVE : CUNode->getGlobalVariables())
+ GVMap[GVE->getVariable()].push_back({nullptr, GVE->getExpression()});
+ DenseSet<DIGlobalVariable *> Processed;
+ for (auto *GVE : CUNode->getGlobalVariables()) {
+ DIGlobalVariable *GV = GVE->getVariable();
+ if (Processed.insert(GV).second)
+ CU.getOrCreateGlobalVariableDIE(GV, sortGlobalExprs(GVMap[GV]));
+ }
+
for (auto *Ty : CUNode->getEnumTypes()) {
// The enum types array by design contains pointers to
// MDNodes rather than DIRefs. Unique them here.
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
index bebf3db..96d9f09 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
@@ -215,6 +215,10 @@
void EmitUnsigned(uint64_t Value) override;
bool isFrameRegister(const TargetRegisterInfo &TRI,
unsigned MachineReg) override;
+ DIELoc *finalize() {
+ DwarfExpression::finalize();
+ return &DIE;
+ }
};
}