<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Sep 6, 2013 at 12:54 PM, 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">On Fri, Sep 6, 2013 at 12:02 PM, Manman Ren <span dir="ltr"><<a href="mailto:manman.ren@gmail.com" target="_blank">manman.ren@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div>On Fri, Sep 6, 2013 at 11:53 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div>On Fri, Sep 6, 2013 at 11:46 AM, Manman Ren <span dir="ltr"><<a href="mailto:manman.ren@gmail.com" target="_blank">manman.ren@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: mren<br>
Date: Fri Sep 6 13:46:00 2013<br>
New Revision: 190190<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=190190&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=190190&view=rev</a><br>
Log:<br>
Debug Info: Use identifier to reference DIType in containing type field of<br>
a DICompositeType.<br>
<br>
Verifier is updated accordingly.<br>
<br>
Added:<br>
llvm/trunk/test/DebugInfo/tu-composite.ll<br>
Modified:<br>
llvm/trunk/include/llvm/DebugInfo.h<br>
llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp<br>
llvm/trunk/lib/IR/DIBuilder.cpp<br>
llvm/trunk/lib/IR/DebugInfo.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/DebugInfo.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo.h?rev=190190&r1=190189&r2=190190&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo.h?rev=190190&r1=190189&r2=190190&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/DebugInfo.h (original)<br>
+++ llvm/trunk/include/llvm/DebugInfo.h Fri Sep 6 13:46:00 2013<br>
@@ -358,8 +358,8 @@ namespace llvm {<br>
void setTypeArray(DIArray Elements, DIArray TParams = DIArray());<br>
void addMember(DIDescriptor D);<br>
unsigned getRunTimeLang() const { return getUnsignedField(11); }<br>
- DICompositeType getContainingType() const {<br>
- return getFieldAs<DICompositeType>(12);<br>
+ DITypeRef getContainingType() const {<br>
+ return getFieldAs<DITypeRef>(12);<br>
}<br>
void setContainingType(DICompositeType ContainingType);<br>
DIArray getTemplateParams() const { return getFieldAs<DIArray>(13); }<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=190190&r1=190189&r2=190190&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=190190&r1=190189&r2=190190&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp Fri Sep 6 13:46:00 2013<br>
@@ -1083,7 +1083,7 @@ void CompileUnit::constructTypeDIE(DIE &<br>
if (CTy.isAppleBlockExtension())<br>
addFlag(&Buffer, dwarf::DW_AT_APPLE_block);<br>
<br>
- DICompositeType ContainingType = CTy.getContainingType();<br>
+ DICompositeType ContainingType(DD->resolve(CTy.getContainingType()));<br>
if (DIDescriptor(ContainingType).isCompositeType())<br>
addDIEEntry(&Buffer, dwarf::DW_AT_containing_type, dwarf::DW_FORM_ref4,<br>
getOrCreateTypeDIE(DIType(ContainingType)));<br>
<br>
Modified: llvm/trunk/lib/IR/DIBuilder.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=190190&r1=190189&r2=190190&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=190190&r1=190189&r2=190190&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/DIBuilder.cpp (original)<br>
+++ llvm/trunk/lib/IR/DIBuilder.cpp Fri Sep 6 13:46:00 2013<br>
@@ -626,7 +626,7 @@ DICompositeType DIBuilder::createClassTy<br>
DerivedFrom,<br>
Elements,<br>
ConstantInt::get(Type::getInt32Ty(VMContext), 0),<br>
- VTableHolder,<br>
+ DIType(VTableHolder).generateRef(),<br></blockquote><div><br></div></div></div><div>Could we make the VTableHolder parameter a DIType so we don't need to cast here (& improve the type-correctness of this function)?</div>
</div></div></div></blockquote></div></div><div>I will try. </div><div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">
<div class="gmail_quote">
<div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
TemplateParams,<br>
UniqueIdentifier.empty() ? NULL : MDString::get(VMContext, UniqueIdentifier)<br>
};<br>
@@ -663,7 +663,7 @@ DICompositeType DIBuilder::createStructT<br>
DerivedFrom,<br>
Elements,<br>
ConstantInt::get(Type::getInt32Ty(VMContext), RunTimeLang),<br>
- VTableHolder,<br>
+ DIType(VTableHolder).generateRef(),<br></blockquote><div><br></div></div><div>and here.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
NULL,<br>
UniqueIdentifier.empty() ? NULL : MDString::get(VMContext, UniqueIdentifier)<br>
};<br>
<br>
Modified: llvm/trunk/lib/IR/DebugInfo.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=190190&r1=190189&r2=190190&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=190190&r1=190189&r2=190190&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/DebugInfo.cpp (original)<br>
+++ llvm/trunk/lib/IR/DebugInfo.cpp Fri Sep 6 13:46:00 2013<br>
@@ -497,7 +497,7 @@ bool DICompositeType::Verify() const {<br>
// Make sure DerivedFrom @ field 9 and ContainingType @ field 12 are MDNodes.<br>
if (!fieldIsMDNode(DbgNode, 9))<br>
return false;<br>
- if (!fieldIsMDNode(DbgNode, 12))<br>
+ if (!fieldIsTypeRef(DbgNode, 12))<br>
return false;<br>
<br>
// Make sure the type identifier at field 14 is MDString, it can be null.<br>
@@ -721,7 +721,7 @@ DITypeRef DIType::generateRef() {<br>
/// \brief Set the containing type.<br>
void DICompositeType::setContainingType(DICompositeType ContainingType) {<br>
TrackingVH<MDNode> N(*this);<br>
- N->replaceOperandWith(12, ContainingType);<br>
+ N->replaceOperandWith(12, ContainingType.generateRef());<br>
DbgNode = N;<br>
}<br>
<br>
<br>
Added: llvm/trunk/test/DebugInfo/tu-composite.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/tu-composite.ll?rev=190190&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/tu-composite.ll?rev=190190&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/DebugInfo/tu-composite.ll (added)<br>
+++ llvm/trunk/test/DebugInfo/tu-composite.ll Fri Sep 6 13:46:00 2013<br></blockquote><div><br></div></div></div><div>Did we not have any tests already covering this functionality? or that could be extended to do so? We generally try not to add new test cases if there's a good home to cover the functionality already.</div>
</div></div></div></blockquote></div></div><div>The problem is that we have testing cases that refer to a DIType via MDNode, but we don't have testing cases that refer to a DIType via the identifier (MDString).</div>
<div>Right now, we support both type references.</div>
<div><br></div><div>To verify that MDString works as a type reference, I added these new testing cases.</div><div><br></div><div>Let me know if we can avoid that.</div></div></div></div></blockquote></div></div><div><br>
We could beef up the verifier to /require/ that any type referenced that has an identifier should be referenced with that identifier (& do this on a per-TypeRef-enabled field, of course - essentially is the "isTypeRef" check, check if the target has an identifier & in that case fail if it's not using that identifier as the typeref). Then we'd find all the test cases that should be exercising this functionality & aren't - update those to use the identifier & now the're actually covering the real production codepath rather than a valid, but less useful one.<br>
</div></div></div></div></blockquote><div>Right now, the existing testing cases all have the identifier field as NULL. To reuse the existing testing cases, we should first set some of the identifier to non-null.</div><div>
<br></div><div>Then we can start using the identifier as a type reference. I will see how hard it is.</div><div><br></div><div>Manman</div></div><br></div></div>