[PATCH] Do not split variables definitions into 2 DIEs.
David Blaikie
dblaikie at gmail.com
Mon Oct 20 14:06:05 PDT 2014
I've checked this out on the GDB test suite & it seems fine.
Looking at the code itself, I think it could be made a bit more
obvious/uniform to have VariableDIE always refer to the definition and have
just a single conditional to decide whether to attach the
DW_AT_specification or not.
Actually this works pretty well & even adds another source fidelity
benefit, allowing the definition, while out-of-line of the class, to be
attributed to the scope in which the definition occurs (eg: namespace x {
struct foo { static int i; }; int foo::i; } - putting the definition in
namespace 'x' instead of just forcing it out to the global namespace) and
exposes a bug in Clang where the definition is scoped to the decl context
instead of the lexical context. (patch attached to show a slightly
short-sighted fix there)
If we did a few more smart things in the frontend, we could get better
fidelity for declarations/definitions of namespace/global variables -
removing the weird special case introduced with my patch. Probably the
right thing here is to optimize in the frontend, if the definition appears
in the decl context (eg: if the global variable's decl context is the same
as the lexical context) then just produce the definition. If the decl and
lexical context differ, produce both a declaration (in the decl context)
and a definition (in the lexical context) - this would be the best source
fidelity, but probably isn't too worthwhile.
Oh, and if we wanted to be really good here, we could make sure we emit any
attributes onto the definition that differ from the declaration - note GCC
does the right thing here, putting DW_AT_decl_line/file on the definitions
if the line/file happens to be different from the declaration's line/file.
We don't do that - we just put nothing on the definition (& I don't think
we do this for function declarations/definitions either (again, here, GCC
gets this right)... could, but probably not important)
On Thu, Oct 16, 2014 at 8:46 AM, Frederic Riss <friss at apple.com> wrote:
> ping
>
> http://reviews.llvm.org/D5457
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141020/ce86c6e0/attachment.html>
-------------- next part --------------
commit a7b341e6d4ba958143efeb21a21d8aaf5c500474
Author: dblaikie <dblaikie at 91177308-0d34-0410-b5e6-96231b3b80d8>
Date: Mon Oct 20 20:29:35 2014 +0000
Fix whitespace introduced in r220221
Post commit review feedback from Yaron Keren.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@220229 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDebugInfo.cpp
index 723da71..489a203 100644
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -1266,7 +1266,7 @@ CollectTemplateParams(const TemplateParameterList *TPList,
// Member function pointers have special support for building them, though
// this is currently unsupported in LLVM CodeGen.
else if ((MD = dyn_cast<CXXMethodDecl>(D)) && MD->isInstance())
- V = CGM.getCXXABI().EmitMemberPointer(MD);
+ V = CGM.getCXXABI().EmitMemberPointer(MD);
else if (const auto *FD = dyn_cast<FunctionDecl>(D))
V = CGM.GetAddrOfFunction(FD);
// Member data pointers have special handling too to compute the fixed
-------------- next part --------------
commit 97e322f831f294b7ce6e063812aa9de8e3b3d076
Author: David Blaikie <dblaikie at gmail.com>
Date: Mon Oct 20 13:20:22 2014 -0700
Avoid excess global variable emission.
diff --git lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index 89d0df2..f05937d 100644
--- lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -105,28 +105,20 @@ DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(DIGlobalVariable GV) {
DIScope GVContext = GV.getContext();
DIType GTy = DD->resolve(GV.getType());
- // If this is a static data member definition, some attributes belong
- // to the declaration DIE.
- DIE *VariableDIE = nullptr;
- bool IsStaticMember = false;
- DIDerivedType SDMDecl = GV.getStaticDataMemberDeclaration();
- if (SDMDecl.Verify()) {
- assert(SDMDecl.isStaticMember() && "Expected static member decl");
- // We need the declaration DIE that is in the static member's class.
- VariableDIE = getOrCreateStaticMemberDIE(SDMDecl);
- IsStaticMember = true;
- }
-
- // If this is not a static data member definition, create the variable
- // DIE and add the initial set of attributes to it.
- if (!VariableDIE) {
- // Construct the context before querying for the existence of the DIE in
- // case such construction creates the DIE.
- DIE *ContextDIE = getOrCreateContextDIE(GVContext);
+ // Construct the context before querying for the existence of the DIE in
+ // case such construction creates the DIE.
+ DIE *ContextDIE = getOrCreateContextDIE(GVContext);
- // Add to map.
- VariableDIE = &createAndAddDIE(GV.getTag(), *ContextDIE, GV);
+ // Add to map.
+ DIE *VariableDIE = &createAndAddDIE(GV.getTag(), *ContextDIE, GV);
+ if (DIDerivedType SDMDecl = GV.getStaticDataMemberDeclaration()) {
+ assert(SDMDecl.isStaticMember() && "Expected static member decl");
+ assert(GV.isDefinition());
+ // We need the declaration DIE that is in the static member's class.
+ DIE *VariableSpecDIE = getOrCreateStaticMemberDIE(SDMDecl);
+ addDIEEntry(*VariableDIE, dwarf::DW_AT_specification, *VariableSpecDIE);
+ } else {
// Add name and type.
addString(*VariableDIE, dwarf::DW_AT_name, GV.getDisplayName());
addType(*VariableDIE, GTy);
@@ -139,9 +131,11 @@ DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(DIGlobalVariable GV) {
addSourceLine(*VariableDIE, GV);
}
+ if (!GV.isDefinition())
+ addFlag(*VariableDIE, dwarf::DW_AT_declaration);
+
// Add location.
bool addToAccelTable = false;
- DIE *VariableSpecDIE = nullptr;
bool isGlobalVariable = GV.getGlobal() != nullptr;
if (isGlobalVariable) {
addToAccelTable = true;
@@ -172,41 +166,21 @@ DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(DIGlobalVariable GV) {
DD->addArangeLabel(SymbolCU(this, Sym));
addOpAddress(*Loc, Sym);
}
- // A static member's declaration is already flagged as such.
- if (!SDMDecl.Verify() && !GV.isDefinition())
- addFlag(*VariableDIE, dwarf::DW_AT_declaration);
- // Do not create specification DIE if context is either compile unit
- // or a subprogram.
- if (GVContext && GV.isDefinition() && !GVContext.isCompileUnit() &&
- !GVContext.isFile() && !DD->isSubprogramContext(GVContext)) {
- // Create specification DIE.
- VariableSpecDIE = &createAndAddDIE(dwarf::DW_TAG_variable, UnitDie);
- addDIEEntry(*VariableSpecDIE, dwarf::DW_AT_specification, *VariableDIE);
- addBlock(*VariableSpecDIE, dwarf::DW_AT_location, Loc);
- // A static member's declaration is already flagged as such.
- if (!SDMDecl.Verify())
- addFlag(*VariableDIE, dwarf::DW_AT_declaration);
- } else {
- addBlock(*VariableDIE, dwarf::DW_AT_location, Loc);
- }
+
+ addBlock(*VariableDIE, dwarf::DW_AT_location, Loc);
// Add the linkage name.
StringRef LinkageName = GV.getLinkageName();
if (!LinkageName.empty())
// From DWARF4: DIEs to which DW_AT_linkage_name may apply include:
// TAG_common_block, TAG_constant, TAG_entry_point, TAG_subprogram and
// TAG_variable.
- addString(IsStaticMember && VariableSpecDIE ? *VariableSpecDIE
- : *VariableDIE,
+ addString(*VariableDIE,
DD->getDwarfVersion() >= 4 ? dwarf::DW_AT_linkage_name
: dwarf::DW_AT_MIPS_linkage_name,
GlobalValue::getRealLinkageName(LinkageName));
} else if (const ConstantInt *CI =
dyn_cast_or_null<ConstantInt>(GV.getConstant())) {
- // AT_const_value was added when the static member was created. To avoid
- // emitting AT_const_value multiple times, we only add AT_const_value when
- // it is not a static member.
- if (!IsStaticMember)
- addConstantValue(*VariableDIE, CI, GTy);
+ addConstantValue(*VariableDIE, CI, GTy);
} else if (const ConstantExpr *CE = getMergedGlobalExpr(GV.getConstant())) {
addToAccelTable = true;
// GV is a merged global.
@@ -223,19 +197,17 @@ DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(DIGlobalVariable GV) {
addBlock(*VariableDIE, dwarf::DW_AT_location, Loc);
}
- DIE *ResultDIE = VariableSpecDIE ? VariableSpecDIE : VariableDIE;
-
if (addToAccelTable) {
- DD->addAccelName(GV.getName(), *ResultDIE);
+ DD->addAccelName(GV.getName(), *VariableDIE);
// If the linkage name is different than the name, go ahead and output
// that as well into the name table.
if (GV.getLinkageName() != "" && GV.getName() != GV.getLinkageName())
- DD->addAccelName(GV.getLinkageName(), *ResultDIE);
+ DD->addAccelName(GV.getLinkageName(), *VariableDIE);
}
- addGlobalName(GV.getName(), *ResultDIE, GV.getContext());
- return ResultDIE;
+ addGlobalName(GV.getName(), *VariableDIE, GV.getContext());
+ return VariableDIE;
}
void DwarfCompileUnit::addRange(RangeSpan Range) {
More information about the llvm-commits
mailing list