[llvm] r204162 - Debug info: Remove OdrMemberMap from DwarfDebug, it's not necessary.
Manman Ren
manman.ren at gmail.com
Fri Mar 21 10:59:47 PDT 2014
On Fri, Mar 21, 2014 at 10:44 AM, David Blaikie <dblaikie at gmail.com> wrote:
> On Fri, Mar 21, 2014 at 10:34 AM, Manman Ren <manman.ren at gmail.com> wrote:
> >
> >
> >
> > On Tue, Mar 18, 2014 at 10:41 AM, Adrian Prantl <aprantl at apple.com>
> wrote:
> >>
> >> Author: adrian
> >> Date: Tue Mar 18 12:41:15 2014
> >> New Revision: 204162
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=204162&view=rev
> >> Log:
> >> Debug info: Remove OdrMemberMap from DwarfDebug, it's not necessary.
> >> Follow-up to r203982.
> >>
> >> Modified:
> >> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> >> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
> >> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> >> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h
> >>
> >> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=204162&r1=204161&r2=204162&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> >> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Tue Mar 18 12:41:15
> >> 2014
> >> @@ -369,7 +369,6 @@ bool DwarfDebug::isSubprogramContext(con
> >> // scope then create and insert DIEs for these variables.
> >> DIE *DwarfDebug::updateSubprogramScopeDIE(DwarfCompileUnit *SPCU,
> >> DISubprogram SP) {
> >> - SP = SPCU->getOdrUniqueSubprogram(resolve(SP.getContext()), SP);
> >> DIE *SPDie = SPCU->getDIE(SP);
> >>
> >> assert(SPDie && "Unable to find subprogram DIE!");
> >> @@ -604,7 +603,8 @@ DIE *DwarfDebug::constructScopeDIE(Dwarf
> >> if (!Scope || !Scope->getScopeNode())
> >> return NULL;
> >>
> >> - DIScope DS(Scope->getScopeNode());
> >> + // Unique scope where applicable.
> >> + DIScope DS(resolve(DIScope(Scope->getScopeNode()).getRef()));
> >
> >
> > Just to recapture what Adrian and I discussed earlier. We try not to
> unique
> > types in the backend. All references to
> > DITypes with an identifier should be converted to DIRef if possible (with
> > the exception of the retained type list).
>
> OK - so I'm a bit confused by this code, then. Are you suggesting that
> "we try not to unique types in the backend, but this case above is an
> exception that we need to do in the backend for <reason>" or are you
> saying that this code isn't an instance of what you would call
> "uniquing types in the backend".
>
Hi David,
I think the answer is the paragraph right below :]
> Adrian tried reverting r203982, the testing case for r203982 will still
> work, it is working because of his earlier commit on converting
DIVariable's
> type to use DIRef.
To clarify, I meant reverting both r203982 and this commit (this commit is
a revert of r203982 + changes), but keeping the testing cases for r203982.
Basically with Adrian's work on converting DIVariable's type to use DIRef,
testing case for r203982 will just work!
Thanks,
Manman
> It looks as if we still want to change this code so it doesn't loop
> back through "uniquing" by getting a ref to the current entity and
> "uniquing" it. Instead we should change getScopeNode to return an
> MDNode or MDString as other "refs" do - and resolve it directly
> (rather than having it reference the MDNode directly, querying that
> for the name, then looking the name up to find (possibly the same)
> MDNode)?
>
> >
> >
> > Adrian tried reverting r203982, the testing case for r203982 will still
> > work, it is working because of his earlier commit on converting
> DIVariable's
> > type to use DIRef.
> >
> >>
> >>
> >> SmallVector<DIE *, 8> Children;
> >> DIE *ObjectPointer = NULL;
> >>
> >> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=204162&r1=204161&r2=204162&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
> >> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Tue Mar 18 12:41:15
> >> 2014
> >> @@ -346,9 +346,6 @@ class DwarfDebug : public AsmPrinterHand
> >> /// of in DwarfCompileUnit.
> >> DenseMap<const MDNode *, DIE *> MDTypeNodeToDieMap;
> >>
> >> - // Used to unique C++ member function declarations.
> >> - StringMap<const MDNode *> OdrMemberMap;
> >> -
> >> // List of all labels used in aranges generation.
> >> std::vector<SymbolCU> ArangeLabels;
> >>
> >> @@ -700,11 +697,6 @@ public:
> >> return MDTypeNodeToDieMap.lookup(TypeMD);
> >> }
> >>
> >> - /// \brief Look up or create an entry in the OdrMemberMap.
> >> - const MDNode *&getOrCreateOdrMember(StringRef Key) {
> >> - return OdrMemberMap.GetOrCreateValue(Key).getValue();
> >> - }
> >> -
> >> /// \brief Emit all Dwarf sections that should come prior to the
> >> /// content.
> >> void beginModule();
> >>
> >> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=204162&r1=204161&r2=204162&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)
> >> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Tue Mar 18 12:41:15
> >> 2014
> >> @@ -1427,24 +1427,6 @@ DIE *DwarfUnit::getOrCreateNameSpace(DIN
> >> return NDie;
> >> }
> >>
> >> -/// Unique C++ member function declarations based on their
> >> -/// context and mangled name.
> >> -DISubprogram
> >> -DwarfUnit::getOdrUniqueSubprogram(DIScope Context, DISubprogram SP)
> const
> >> {
> >> - if (!hasODR() ||
> >> - !Context.isCompositeType() ||
> >> - SP.getLinkageName().empty() ||
> >> - SP.isDefinition())
> >> - return SP;
> >> - // Create a key with the UID of the parent class and this SP's name.
> >> - Twine Key = SP.getContext().getName() + SP.getLinkageName();
> >> - const MDNode *&Entry = DD->getOrCreateOdrMember(Key.str());
> >> - if (!Entry)
> >> - Entry = &*SP;
> >> -
> >> - return DISubprogram(Entry);
> >> -}
> >> -
> >> /// getOrCreateSubprogramDIE - Create new DIE using SP.
> >> DIE *DwarfUnit::getOrCreateSubprogramDIE(DISubprogram SP) {
> >> // Construct the context before querying for the existence of the DIE
> >> in case
> >> @@ -1452,8 +1434,10 @@ DIE *DwarfUnit::getOrCreateSubprogramDIE
> >> // declarations).
> >> DIScope Context = resolve(SP.getContext());
> >> DIE *ContextDIE = getOrCreateContextDIE(Context);
> >> +
> >> // Unique declarations based on the ODR, where applicable.
> >> - SP = getOdrUniqueSubprogram(Context, SP);
> >> + SP = DISubprogram(DD->resolve(SP.getRef()));
> >
> >
> > This line of code does nothing, since we don't have identifiers for
> > DISubprogram.
>
> Nice catch - yeah... this is more of that loop-back "uniquing" anyway
> - it'd be nice to get rid of any code like this in favor of fixing the
> ways objects are referenced. (I understand there's a reasonable
> exception when visiting the retained types list, potentially - though
> we could think about that a bit further too)
>
> So we should remove this line of code, right?
>
> - David
>
> >
> > Thanks,
> > Manman
> >
> >>
> >> + assert(SP.Verify());
> >>
> >> DIE *SPDie = getDIE(SP);
> >> if (SPDie)
> >>
> >> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h?rev=204162&r1=204161&r2=204162&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h (original)
> >> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h Tue Mar 18 12:41:15
> 2014
> >> @@ -487,28 +487,6 @@ public:
> >>
> >> virtual DwarfCompileUnit &getCU() = 0;
> >>
> >> - /// \brief Return whether this compilation unit has the
> >> - /// one-definition-rule (ODR). In C++ this allows the compiler to
> >> - /// perform type unique during LTO.
> >> - bool hasODR() const {
> >> - switch (getLanguage()) {
> >> - case dwarf::DW_LANG_C_plus_plus:
> >> - case dwarf::DW_LANG_C_plus_plus_03:
> >> - case dwarf::DW_LANG_C_plus_plus_11:
> >> - // For all we care, the C++ part of the language has the ODR and
> >> - // ObjC methods are not represented in a way that they could be
> >> - // confused with C++ member functions.
> >> - case dwarf::DW_LANG_ObjC_plus_plus:
> >> - return true;
> >> - default:
> >> - return false;
> >> - }
> >> - }
> >> -
> >> - /// \brief Unique C++ member function declarations based on their
> >> - /// context+mangled name.
> >> - DISubprogram getOdrUniqueSubprogram(DIScope Context, DISubprogram SP)
> >> const;
> >> -
> >> protected:
> >> /// getOrCreateStaticMemberDIE - Create new static data member DIE.
> >> DIE *getOrCreateStaticMemberDIE(DIDerivedType DT);
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
> >
> > _______________________________________________
> > 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/20140321/251c936e/attachment.html>
More information about the llvm-commits
mailing list