[llvm] r191792 - Debug Info: remove duplication of DIEs when a DIE is part of the type system
David Blaikie
dblaikie at gmail.com
Tue Oct 1 13:57:36 PDT 2013
On Tue, Oct 1, 2013 at 12:52 PM, Manman Ren <manman.ren at gmail.com> wrote:
> Author: mren
> Date: Tue Oct 1 14:52:23 2013
> New Revision: 191792
>
> URL: http://llvm.org/viewvc/llvm-project?rev=191792&view=rev
> Log:
> Debug Info: remove duplication of DIEs when a DIE is part of the type
> system
> and it is shared across CUs.
>
This may require some discussion/consideration.
The first thing that comes to my mind is that this is changing the DIE
structure from trees to DAGs and, perhaps importantly, is going to make
parent/child relationships incomplete (ie: A child of B no longer implies
that B is the parent of A) which is a bit concerning.
>
> We add a few maps in DwarfDebug to map MDNodes for the type system to the
> corresponding DIEs: MDTypeNodeToDieMap, MDSPNodeToDieMap, and
> MDStaticMemberNodeToDieMap. These DIEs can be shared across CUs, that is
> why we
> keep the maps in DwarfDebug instead of CompileUnit.
>
Why nod simply map MDNode* to DIE the same as we do in the CompileUnit? We
should be able to do this for all DIEs, no? (essentially just move the map
up from CompileUnit to DwarfDebug)
But I'm not sure how effective this would be - like I said, might need some
thought/discussion to understand your solution & consider alternatives.
>
> Sometimes, when we try to add an attribute to a DIE, the DIE is not yet
> added
> to its owner yet, so we don't know whether we should use ref_addr or ref4.
> We create a worklist that will be processed during finalization to add
> attributes with the correct form (ref_addr or ref4).
>
Do you have an example of this as a test case? (I really wish we had an
easy way to see test coverage - so I could just tell at a glance whether
the worklist code is being exercised by your test cases... oh well)
>
> We add addDIEEntry to DwarfDebug to be a wrapper around DIE->addValue. It
> checks
> whether we know the correct form, if not, we update the worklist
> (DIEEntryWorklist).
>
> A testing case is added to show that we only create a single DIE for a type
> MDNode and we use ref_addr to refer to the type DIE.
>
> Modified:
> llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
> llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h
> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
> llvm/trunk/test/Linker/type-unique-simple-a.ll
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp?rev=191792&r1=191791&r2=191792&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp Tue Oct 1 14:52:23 2013
> @@ -113,13 +113,21 @@ DIE::~DIE() {
> /// Climb up the parent chain to get the compile unit DIE to which this
> DIE
> /// belongs.
> DIE *DIE::getCompileUnit() {
> + DIE *Cu = checkCompileUnit();
> + assert(Cu && "We should not have orphaned DIEs.");
> + return Cu;
> +}
> +
> +/// Climb up the parent chain to get the compile unit DIE this DIE belongs
> +/// to. Return NULL if DIE is not added to an owner yet.
> +DIE *DIE::checkCompileUnit() {
> DIE *p = this;
> while (p) {
> if (p->getTag() == dwarf::DW_TAG_compile_unit)
> return p;
> p = p->getParent();
> }
> - llvm_unreachable("We should not have orphaned DIEs.");
> + return NULL;
}
>
> DIEValue *DIE::findAttribute(uint16_t Attribute) {
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h?rev=191792&r1=191791&r2=191792&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h Tue Oct 1 14:52:23 2013
> @@ -152,6 +152,9 @@ namespace llvm {
> /// Climb up the parent chain to get the compile unit DIE this DIE
> belongs
> /// to.
> DIE *getCompileUnit();
> + /// Similar to getCompileUnit, returns null when DIE is not added to
> an
> + /// owner yet.
> + DIE *checkCompileUnit();
>
Why is this not just "getParent() == NULL"? Why do you need a walk for the
DIE?
> void setTag(uint16_t Tag) { Abbrev.setTag(Tag); }
> void setOffset(unsigned O) { Offset = O; }
> void setSize(unsigned S) { Size = S; }
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=191792&r1=191791&r2=191792&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp Tue Oct 1
> 14:52:23 2013
> @@ -242,7 +242,7 @@ void CompileUnit::addDelta(DIE *Die, uin
> ///
> void CompileUnit::addDIEEntry(DIE *Die, uint16_t Attribute, uint16_t Form,
> DIE *Entry) {
> - Die->addValue(Attribute, Form, createDIEEntry(Entry));
> + DD->addDIEEntry(Die, Attribute, Form, createDIEEntry(Entry));
> }
>
> /// addBlock - Add block data.
> @@ -784,13 +784,13 @@ DIE *CompileUnit::getOrCreateTypeDIE(con
> DIType Ty(TyNode);
> if (!Ty.isType())
> return NULL;
> - DIE *TyDIE = getDIE(Ty);
> + DIE *TyDIE = DD->getTypeDIE(Ty);
> if (TyDIE)
> return TyDIE;
>
> // Create new type.
> TyDIE = new DIE(dwarf::DW_TAG_base_type);
> - insertDIE(Ty, TyDIE);
> + DD->insertTypeDIE(Ty, TyDIE);
> if (Ty.isBasicType())
> constructTypeDIE(*TyDIE, DIBasicType(Ty));
> else if (Ty.isCompositeType())
> @@ -826,7 +826,7 @@ void CompileUnit::addType(DIE *Entity, D
> DIEEntry *Entry = getDIEEntry(Ty);
> // If it exists then use the existing value.
> if (Entry) {
> - Entity->addValue(Attribute, dwarf::DW_FORM_ref4, Entry);
> + DD->addDIEEntry(Entity, Attribute, dwarf::DW_FORM_ref4, Entry);
> return;
> }
>
> @@ -836,7 +836,7 @@ void CompileUnit::addType(DIE *Entity, D
> // Set up proxy.
> Entry = createDIEEntry(Buffer);
> insertDIEEntry(Ty, Entry);
> - Entity->addValue(Attribute, dwarf::DW_FORM_ref4, Entry);
> + DD->addDIEEntry(Entity, Attribute, dwarf::DW_FORM_ref4, Entry);
>
> // If this is a complete composite type then include it in the
> // list of global types.
> @@ -1268,14 +1268,14 @@ DIE *CompileUnit::getOrCreateNameSpace(D
>
> /// getOrCreateSubprogramDIE - Create new DIE using SP.
> DIE *CompileUnit::getOrCreateSubprogramDIE(DISubprogram SP) {
> - DIE *SPDie = getDIE(SP);
> + DIE *SPDie = DD->getSPDIE(SP);
> if (SPDie)
> return SPDie;
>
> SPDie = new DIE(dwarf::DW_TAG_subprogram);
>
> // DW_TAG_inlined_subroutine may refer to this DIE.
> - insertDIE(SP, SPDie);
> + DD->insertSPDIE(SP, SPDie);
>
> DISubprogram SPDecl = SP.getFunctionDeclaration();
> DIE *DeclDie = NULL;
> @@ -1422,7 +1422,7 @@ void CompileUnit::createGlobalVariableDI
> // But that class might not exist in the DWARF yet.
> // Creating the class will create the static member decl DIE.
> getOrCreateContextDIE(DD->resolve(SDMDecl.getContext()));
> - VariableDIE = getDIE(SDMDecl);
> + VariableDIE = DD->getStaticMemberDIE(SDMDecl);
> assert(VariableDIE && "Static member decl has no context?");
> IsStaticMember = true;
> }
> @@ -1616,7 +1616,7 @@ void CompileUnit::constructContainingTyp
> DIE *SPDie = CI->first;
> const MDNode *N = CI->second;
> if (!N) continue;
> - DIE *NDie = getDIE(N);
> + DIE *NDie = DD->getTypeDIE(N);
> if (!NDie) continue;
> addDIEEntry(SPDie, dwarf::DW_AT_containing_type, dwarf::DW_FORM_ref4,
> NDie);
> }
> @@ -1819,6 +1819,6 @@ DIE *CompileUnit::createStaticMemberDIE(
> if (const ConstantFP *CFP =
> dyn_cast_or_null<ConstantFP>(DT.getConstant()))
> addConstantFPValue(StaticMemberDIE, CFP);
>
> - insertDIE(DT, StaticMemberDIE);
> + DD->insertStaticMemberDIE(DT, StaticMemberDIE);
> return StaticMemberDIE;
> }
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=191792&r1=191791&r2=191792&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Tue Oct 1 14:52:23
> 2013
> @@ -357,7 +357,7 @@ bool DwarfDebug::isSubprogramContext(con
> // scope then create and insert DIEs for these variables.
> DIE *DwarfDebug::updateSubprogramScopeDIE(CompileUnit *SPCU,
> const MDNode *SPNode) {
> - DIE *SPDie = SPCU->getDIE(SPNode);
> + DIE *SPDie = getSPDIE(SPNode);
>
> assert(SPDie && "Unable to find subprogram DIE!");
> DISubprogram SP(SPNode);
> @@ -511,7 +511,7 @@ DIE *DwarfDebug::constructInlinedScopeDI
> return NULL;
> DIScope DS(Scope->getScopeNode());
> DISubprogram InlinedSP = getDISubprogram(DS);
> - DIE *OriginDIE = TheCU->getDIE(InlinedSP);
> + DIE *OriginDIE = getSPDIE(InlinedSP);
> if (!OriginDIE) {
> DEBUG(dbgs() << "Unable to find original DIE for an inlined
> subprogram.");
> return NULL;
> @@ -616,7 +616,7 @@ DIE *DwarfDebug::constructScopeDIE(Compi
> else if (DS.isSubprogram()) {
> ProcessedSPNodes.insert(DS);
> if (Scope->isAbstractScope()) {
> - ScopeDIE = TheCU->getDIE(DS);
> + ScopeDIE = getSPDIE(DS);
> // Note down abstract DIE.
> if (ScopeDIE)
> AbstractSPDies.insert(std::make_pair(DS, ScopeDIE));
> @@ -992,7 +992,7 @@ void DwarfDebug::collectDeadVariables()
> CompileUnit *SPCU = CUMap.lookup(TheCU);
> assert(SPCU && "Unable to find Compile Unit!");
> constructSubprogramDIE(SPCU, SP);
> - DIE *ScopeDIE = SPCU->getDIE(SP);
> + DIE *ScopeDIE = getSPDIE(SP);
> for (unsigned vi = 0, ve = Variables.getNumElements(); vi != ve;
> ++vi) {
> DIVariable DV(Variables.getElement(vi));
> if (!DV.isVariable()) continue;
> @@ -1065,6 +1065,15 @@ void DwarfDebug::finalizeModuleInfo() {
> Hash.computeDIEODRSignature(Die));
> }
>
> + // Process the worklist to add attributes with the correct form
> (ref_addr or
> + // ref4).
> + for (unsigned I = 0, E = DIEEntryWorklist.size(); I < E; I++) {
> + addDIEEntry(DIEEntryWorklist[I].Die, DIEEntryWorklist[I].Attribute,
> + dwarf::DW_FORM_ref4, DIEEntryWorklist[I].Entry);
> + assert(E == DIEEntryWorklist.size() &&
> + "We should not add to the worklist during finalization.");
> + }
> +
> // Handle anything that needs to be done on a per-cu basis.
> for (DenseMap<const MDNode *, CompileUnit *>::iterator CUI =
> CUMap.begin(),
> CUE =
> CUMap.end();
> @@ -2042,7 +2051,11 @@ void DwarfDebug::emitDIE(DIE *Die, std::
> Asm->OutStreamer.AddComment(dwarf::AttributeString(Attr));
>
> switch (Attr) {
> - case dwarf::DW_AT_abstract_origin: {
> + case dwarf::DW_AT_abstract_origin:
> + case dwarf::DW_AT_type:
> + case dwarf::DW_AT_friend:
> + case dwarf::DW_AT_specification:
> + case dwarf::DW_AT_containing_type: {
> DIEEntry *E = cast<DIEEntry>(Values[i]);
> DIE *Origin = E->getEntry();
> unsigned Addr = Origin->getOffset();
> @@ -3031,3 +3044,24 @@ void DwarfDebug::emitDebugStrDWO() {
>
> InfoHolder.emitStrings(Asm->getObjFileLowering().getDwarfStrDWOSection(),
> OffSec, StrSym);
> }
> +
> +/// When we don't know whether the correct form is ref4 or ref_addr, we
> create
> +/// a worklist item and insert it to DIEEntryWorklist.
> +void DwarfDebug::addDIEEntry(DIE *Die, uint16_t Attribute, uint16_t Form,
> + DIEEntry *Entry) {
> + /// Early exit when we only have a single CU.
> + if (GlobalCUIndexCount == 1 || Form != dwarf::DW_FORM_ref4) {
> + Die->addValue(Attribute, Form, Entry);
> + return;
> + }
> + DIE *DieCU = Die->checkCompileUnit();
> + DIE *EntryCU = Entry->getEntry()->checkCompileUnit();
> + if (!DieCU || !EntryCU) {
> + // Die or Entry is not added to an owner yet.
> + insertDIEEntryWorklist(Die, Attribute, Entry);
> + return;
> + }
> + Die->addValue(Attribute,
> + EntryCU == DieCU ? dwarf::DW_FORM_ref4 : dwarf::DW_FORM_ref_addr,
> + Entry);
> +}
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=191792&r1=191791&r2=191792&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Tue Oct 1 14:52:23 2013
> @@ -326,6 +326,30 @@ class DwarfDebug {
> // Maps subprogram MDNode with its corresponding CompileUnit.
> DenseMap <const MDNode *, CompileUnit *> SPMap;
>
> + /// Maps type MDNode with its corresponding DIE. These DIEs can be
> + /// shared across CUs, that is why we keep the map here instead
> + /// of in CompileUnit.
> + DenseMap<const MDNode *, DIE *> MDTypeNodeToDieMap;
> + /// Maps subprogram MDNode with its corresponding DIE.
> + DenseMap<const MDNode *, DIE *> MDSPNodeToDieMap;
> + /// Maps static member MDNode with its corresponding DIE.
> + DenseMap<const MDNode *, DIE *> MDStaticMemberNodeToDieMap;
> +
> + /// Specifies a worklist item. Sometimes, when we try to add an
> attribute to
> + /// a DIE, the DIE is not yet added to its owner yet, so we don't know
> whether
> + /// we should use ref_addr or ref4. We create a worklist that will be
> + /// processed during finalization to add attributes with the correct
> form
> + /// (ref_addr or ref4).
> + struct DIEEntryWorkItem {
> + DIE *Die;
> + uint16_t Attribute;
> + DIEEntry *Entry;
> + DIEEntryWorkItem(DIE *D, uint16_t A, DIEEntry *E) :
> + Die(D), Attribute(A), Entry(E) {
> + }
> + };
> + SmallVector<DIEEntryWorkItem, 64> DIEEntryWorklist;
> +
> // Used to uniquely define abbreviations.
> FoldingSet<DIEAbbrev> AbbreviationsSet;
>
> @@ -660,6 +684,28 @@ public:
> DwarfDebug(AsmPrinter *A, Module *M);
> ~DwarfDebug();
>
> + void insertTypeDIE(const MDNode *TypeMD, DIE *Die) {
> + MDTypeNodeToDieMap.insert(std::make_pair(TypeMD, Die));
> + }
> + DIE *getTypeDIE(const MDNode *TypeMD) {
> + return MDTypeNodeToDieMap.lookup(TypeMD);
> + }
> + void insertSPDIE(const MDNode *SPMD, DIE *Die) {
> + MDSPNodeToDieMap.insert(std::make_pair(SPMD, Die));
> + }
> + DIE *getSPDIE(const MDNode *SPMD) {
> + return MDSPNodeToDieMap.lookup(SPMD);
> + }
> + void insertStaticMemberDIE(const MDNode *StaticMD, DIE *Die) {
> + MDStaticMemberNodeToDieMap.insert(std::make_pair(StaticMD, Die));
> + }
> + DIE *getStaticMemberDIE(const MDNode *StaticMD) {
> + return MDStaticMemberNodeToDieMap.lookup(StaticMD);
> + }
> + void insertDIEEntryWorklist(DIE *Die, uint16_t Attribute, DIEEntry
> *Entry) {
> + DIEEntryWorklist.push_back(DIEEntryWorkItem(Die, Attribute, Entry));
> + }
> +
> /// \brief Emit all Dwarf sections that should come prior to the
> /// content.
> void beginModule();
> @@ -722,6 +768,11 @@ public:
> return Ref.resolve(TypeIdentifierMap);
> }
>
> + /// When we don't know whether the correct form is ref4 or ref_addr, we
> create
> + /// a worklist item and insert it to DIEEntryWorklist.
> + void addDIEEntry(DIE *Die, uint16_t Attribute, uint16_t Form,
> + DIEEntry *Entry);
> +
> /// isSubprogramContext - Return true if Context is either a subprogram
> /// or another context nested inside a subprogram.
> bool isSubprogramContext(const MDNode *Context);
>
> Modified: llvm/trunk/test/Linker/type-unique-simple-a.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/type-unique-simple-a.ll?rev=191792&r1=191791&r2=191792&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Linker/type-unique-simple-a.ll (original)
> +++ llvm/trunk/test/Linker/type-unique-simple-a.ll Tue Oct 1 14:52:23 2013
> @@ -1,7 +1,22 @@
> -; RUN: llvm-link %s %p/type-unique-simple-b.ll -S -o - | FileCheck %s
> +; REQUIRES: object-emission
>
> -; CHECK: DW_TAG_structure_type
> +; RUN: llvm-link %s %p/type-unique-simple-b.ll -S -o %t
> +; RUN: cat %t | FileCheck %s -check-prefix=LINK
> +; RUN: llc -filetype=obj -O0 < %t > %t2
> +; RUN: llvm-dwarfdump -debug-dump=info %t2 | FileCheck %s
> +; CHECK: 0x[[INT:.*]]: DW_TAG_base_type
> +; CHECK-NEXT: DW_AT_name {{.*}} = "int"
> +; CHECK-NOT: DW_TAG_base_type
> +; CHECK: 0x[[BASE:.*]]: DW_TAG_structure_type
> +; CHECK-NEXT: DW_AT_name {{.*}} = "Base"
> ; CHECK-NOT: DW_TAG_structure_type
> +; CHECK: DW_TAG_formal_parameter
> +; CHECK: DW_AT_type [DW_FORM_ref_addr] {{.*}}[[INT]])
> +; CHECK: DW_TAG_variable
> +; CHECK: DW_AT_type [DW_FORM_ref_addr] {{.*}}[[BASE]])
> +
> +; LINK: DW_TAG_structure_type
> +; LINK-NOT: DW_TAG_structure_type
> ; Content of header files:
> ; struct Base {
> ; int a;
>
>
> _______________________________________________
> 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/20131001/b9c81757/attachment.html>
More information about the llvm-commits
mailing list