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