<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jul 28, 2014 at 12:40 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 class="">On Mon, Jul 28, 2014 at 11:52 AM, Manman Ren <<a href="mailto:manman.ren@gmail.com">manman.ren@gmail.com</a>> wrote:<br>
> Author: mren<br>
> Date: Mon Jul 28 13:52:30 2014<br>
> New Revision: 214111<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=214111&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=214111&view=rev</a><br>
> Log:<br>
> [Debug Info] replace DIUnspecifiedParameter with DITrivialType.<br>
><br>
> This is the first of a series of patches to handle type uniqueing of the<br>
> type array for a subroutine type.<br>
><br>
> This commit makes sure unspecified_parameter is a DIType to enable converting<br>
> the type array for a subroutine type to an array of DITypes.<br>
<br>
</div>Can you remind me (& it'll be helpful to have it in this thread for<br>
later reference) why unspecified type couldn't be just a plain DIType<br>
(or DIBasicType, etc) and needed it's own derived class?<br></blockquote><div><br></div><div>Yes, it is mainly for size reasons. But you are right, we will only have one unspecified_type and one unspecified_parameter for each module.</div>
<div>The other reason is for clarity so we know these two are trivial types without size, offset etc.</div><div><br></div><div>I am okay with removing DITrivialType if it is a burden to have one more class in our DIType hierarchy.</div>
<div><br></div><div>Thanks,</div><div>Manman</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I'm guessing it's for size reasons, though I don't think it's too<br>
important to make unspecified_type particularly compact - we'll only<br>
ever need one of them anyway, so we just pay the cost once per module.<br>
We could still have the same assertions in DIType for usability,<br>
though (but they're probably not necessary either - many of DIType's<br>
members aren't valid/meaningful for various types).<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> This commit should have no functionality change. With this commit, we may<br>
> change unspecified type to be a DITrivialType instead of a DIType.<br>
><br>
> Modified:<br>
> llvm/trunk/include/llvm/IR/DIBuilder.h<br>
> llvm/trunk/include/llvm/IR/DebugInfo.h<br>
> llvm/trunk/lib/IR/DIBuilder.cpp<br>
> llvm/trunk/lib/IR/DebugInfo.cpp<br>
><br>
> Modified: llvm/trunk/include/llvm/IR/DIBuilder.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DIBuilder.h?rev=214111&r1=214110&r2=214111&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DIBuilder.h?rev=214111&r1=214110&r2=214111&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/IR/DIBuilder.h (original)<br>
> +++ llvm/trunk/include/llvm/IR/DIBuilder.h Mon Jul 28 13:52:30 2014<br>
> @@ -463,9 +463,9 @@ namespace llvm {<br>
> /// through debug info anchors.<br>
> void retainType(DIType T);<br>
><br>
> - /// createUnspecifiedParameter - Create unspecified type descriptor<br>
> + /// createUnspecifiedParameter - Create unspecified parameter type<br>
> /// for a subroutine type.<br>
> - DIDescriptor createUnspecifiedParameter();<br>
> + DITrivialType createUnspecifiedParameter();<br>
><br>
> /// getOrCreateArray - Get a DIArray, create one if required.<br>
> DIArray getOrCreateArray(ArrayRef<Value *> Elements);<br>
><br>
> Modified: llvm/trunk/include/llvm/IR/DebugInfo.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfo.h?rev=214111&r1=214110&r2=214111&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfo.h?rev=214111&r1=214110&r2=214111&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/IR/DebugInfo.h (original)<br>
> +++ llvm/trunk/include/llvm/IR/DebugInfo.h Mon Jul 28 13:52:30 2014<br>
> @@ -128,6 +128,7 @@ public:<br>
> bool isDerivedType() const;<br>
> bool isCompositeType() const;<br>
> bool isBasicType() const;<br>
> + bool isTrivialType() const;<br>
> bool isVariable() const;<br>
> bool isSubprogram() const;<br>
> bool isGlobalVariable() const;<br>
> @@ -302,15 +303,36 @@ public:<br>
> /// Verify - Verify that a type descriptor is well formed.<br>
> bool Verify() const;<br>
><br>
> - DIScopeRef getContext() const { return getFieldAs<DIScopeRef>(2); }<br>
> - StringRef getName() const { return getStringField(3); }<br>
> - unsigned getLineNumber() const { return getUnsignedField(4); }<br>
> - uint64_t getSizeInBits() const { return getUInt64Field(5); }<br>
> - uint64_t getAlignInBits() const { return getUInt64Field(6); }<br>
> + DIScopeRef getContext() const {<br>
> + assert(!isTrivialType() && "no context for DITrivialType");<br>
> + return getFieldAs<DIScopeRef>(2);<br>
> + }<br>
> + StringRef getName() const {<br>
> + assert(!isTrivialType() && "no name for DITrivialType");<br>
> + return getStringField(3);<br>
> + }<br>
> + unsigned getLineNumber() const {<br>
> + assert(!isTrivialType() && "no line number for DITrivialType");<br>
> + return getUnsignedField(4);<br>
> + }<br>
> + uint64_t getSizeInBits() const {<br>
> + assert(!isTrivialType() && "no SizeInBits for DITrivialType");<br>
> + return getUInt64Field(5);<br>
> + }<br>
> + uint64_t getAlignInBits() const {<br>
> + assert(!isTrivialType() && "no AlignInBits for DITrivialType");<br>
> + return getUInt64Field(6);<br>
> + }<br>
> // FIXME: Offset is only used for DW_TAG_member nodes. Making every type<br>
> // carry this is just plain insane.<br>
> - uint64_t getOffsetInBits() const { return getUInt64Field(7); }<br>
> - unsigned getFlags() const { return getUnsignedField(8); }<br>
> + uint64_t getOffsetInBits() const {<br>
> + assert(!isTrivialType() && "no OffsetInBits for DITrivialType");<br>
> + return getUInt64Field(7);<br>
> + }<br>
> + unsigned getFlags() const {<br>
> + assert(!isTrivialType() && "no flag for DITrivialType");<br>
> + return getUnsignedField(8);<br>
> + }<br>
> bool isPrivate() const { return (getFlags() & FlagPrivate) != 0; }<br>
> bool isProtected() const { return (getFlags() & FlagProtected) != 0; }<br>
> bool isForwardDecl() const { return (getFlags() & FlagFwdDecl) != 0; }<br>
> @@ -343,6 +365,12 @@ public:<br>
> void replaceAllUsesWith(MDNode *D);<br>
> };<br>
><br>
> +class DITrivialType : public DIType {<br>
> +public:<br>
> + explicit DITrivialType(const MDNode *N = nullptr) : DIType(N) {}<br>
> + bool Verify() const;<br>
> +};<br>
> +<br>
> /// DIBasicType - A basic type, like 'int' or 'float'.<br>
> class DIBasicType : public DIType {<br>
> public:<br>
> @@ -571,14 +599,6 @@ public:<br>
> bool Verify() const;<br>
> };<br>
><br>
> -/// DIUnspecifiedParameter - This is a wrapper for unspecified parameters.<br>
> -class DIUnspecifiedParameter : public DIDescriptor {<br>
> -public:<br>
> - explicit DIUnspecifiedParameter(const MDNode *N = nullptr)<br>
> - : DIDescriptor(N) {}<br>
> - bool Verify() const;<br>
> -};<br>
> -<br>
> /// DITemplateTypeParameter - This is a wrapper for template type parameter.<br>
> class DITemplateTypeParameter : public DIDescriptor {<br>
> public:<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=214111&r1=214110&r2=214111&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=214111&r1=214110&r2=214111&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/IR/DIBuilder.cpp (original)<br>
> +++ llvm/trunk/lib/IR/DIBuilder.cpp Mon Jul 28 13:52:30 2014<br>
> @@ -875,11 +875,11 @@ void DIBuilder::retainType(DIType T) {<br>
><br>
> /// createUnspecifiedParameter - Create unspeicified type descriptor<br>
> /// for the subroutine type.<br>
> -DIDescriptor DIBuilder::createUnspecifiedParameter() {<br>
> +DITrivialType DIBuilder::createUnspecifiedParameter() {<br>
> Value *Elts[] = {<br>
> GetTagConstant(VMContext, dwarf::DW_TAG_unspecified_parameters)<br>
> };<br>
> - return DIDescriptor(MDNode::get(VMContext, Elts));<br>
> + return DITrivialType(MDNode::get(VMContext, Elts));<br>
> }<br>
><br>
> /// createForwardDecl - Create a temporary forward-declared type that<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=214111&r1=214110&r2=214111&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=214111&r1=214110&r2=214111&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/IR/DebugInfo.cpp (original)<br>
> +++ llvm/trunk/lib/IR/DebugInfo.cpp Mon Jul 28 13:52:30 2014<br>
> @@ -39,6 +39,7 @@ bool DIDescriptor::Verify() const {<br>
> return DbgNode &&<br>
> (DIDerivedType(DbgNode).Verify() ||<br>
> DICompositeType(DbgNode).Verify() || DIBasicType(DbgNode).Verify() ||<br>
> + DITrivialType(DbgNode).Verify() ||<br>
> DIVariable(DbgNode).Verify() || DISubprogram(DbgNode).Verify() ||<br>
> DIGlobalVariable(DbgNode).Verify() || DIFile(DbgNode).Verify() ||<br>
> DICompileUnit(DbgNode).Verify() || DINameSpace(DbgNode).Verify() ||<br>
> @@ -46,7 +47,6 @@ bool DIDescriptor::Verify() const {<br>
> DILexicalBlockFile(DbgNode).Verify() ||<br>
> DISubrange(DbgNode).Verify() || DIEnumerator(DbgNode).Verify() ||<br>
> DIObjCProperty(DbgNode).Verify() ||<br>
> - DIUnspecifiedParameter(DbgNode).Verify() ||<br>
> DITemplateTypeParameter(DbgNode).Verify() ||<br>
> DITemplateValueParameter(DbgNode).Verify() ||<br>
> DIImportedEntity(DbgNode).Verify());<br>
> @@ -155,6 +155,10 @@ MDNode *DIVariable::getInlinedAt() const<br>
> // Predicates<br>
> //===----------------------------------------------------------------------===//<br>
><br>
> +bool DIDescriptor::isTrivialType() const {<br>
> + return DbgNode && getTag() == dwarf::DW_TAG_unspecified_parameters;<br>
> +}<br>
> +<br>
> /// isBasicType - Return true if the specified tag is legal for<br>
> /// DIBasicType.<br>
> bool DIDescriptor::isBasicType() const {<br>
> @@ -225,7 +229,8 @@ bool DIDescriptor::isVariable() const {<br>
><br>
> /// isType - Return true if the specified tag is legal for DIType.<br>
> bool DIDescriptor::isType() const {<br>
> - return isBasicType() || isCompositeType() || isDerivedType();<br>
> + return isBasicType() || isCompositeType() || isDerivedType() ||<br>
> + isTrivialType();<br>
> }<br>
><br>
> /// isSubprogram - Return true if the specified tag is legal for<br>
> @@ -456,7 +461,7 @@ bool DIType::Verify() const {<br>
><br>
> // FIXME: Sink this into the various subclass verifies.<br>
> uint16_t Tag = getTag();<br>
> - if (!isBasicType() && Tag != dwarf::DW_TAG_const_type &&<br>
> + if (!isBasicType() && !isTrivialType() && Tag != dwarf::DW_TAG_const_type &&<br>
> Tag != dwarf::DW_TAG_volatile_type && Tag != dwarf::DW_TAG_pointer_type &&<br>
> Tag != dwarf::DW_TAG_ptr_to_member_type &&<br>
> Tag != dwarf::DW_TAG_reference_type &&<br>
> @@ -471,6 +476,8 @@ bool DIType::Verify() const {<br>
> // a CompositeType.<br>
> if (isBasicType())<br>
> return DIBasicType(DbgNode).Verify();<br>
> + else if (isTrivialType())<br>
> + return DITrivialType(DbgNode).Verify();<br>
> else if (isCompositeType())<br>
> return DICompositeType(DbgNode).Verify();<br>
> else if (isDerivedType())<br>
> @@ -484,6 +491,10 @@ bool DIBasicType::Verify() const {<br>
> return isBasicType() && DbgNode->getNumOperands() == 10;<br>
> }<br>
><br>
> +bool DITrivialType::Verify() const {<br>
> + return isTrivialType() && DbgNode->getNumOperands() == 1;<br>
> +}<br>
> +<br>
> /// Verify - Verify that a derived type descriptor is well formed.<br>
> bool DIDerivedType::Verify() const {<br>
> // Make sure DerivedFrom @ field 9 is TypeRef.<br>
> @@ -624,11 +635,6 @@ bool DILexicalBlockFile::Verify() const<br>
> return isLexicalBlockFile() && DbgNode->getNumOperands() == 3;<br>
> }<br>
><br>
> -/// \brief Verify that an unspecified parameter descriptor is well formed.<br>
> -bool DIUnspecifiedParameter::Verify() const {<br>
> - return isUnspecifiedParameter() && DbgNode->getNumOperands() == 1;<br>
> -}<br>
> -<br>
> /// \brief Verify that the template type parameter descriptor is well formed.<br>
> bool DITemplateTypeParameter::Verify() const {<br>
> return isTemplateTypeParameter() && DbgNode->getNumOperands() == 7;<br>
> @@ -1290,7 +1296,7 @@ void DIEnumerator::printInternal(raw_ost<br>
> }<br>
><br>
> void DIType::printInternal(raw_ostream &OS) const {<br>
> - if (!DbgNode)<br>
> + if (!DbgNode || isTrivialType())<br>
> return;<br>
><br>
> StringRef Res = getName();<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>
</div></div></blockquote></div><br></div></div>