[llvm] r214111 - [Debug Info] replace DIUnspecifiedParameter with DITrivialType.
Manman Ren
manman.ren at gmail.com
Mon Jul 28 17:33:37 PDT 2014
On Mon, Jul 28, 2014 at 4:19 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Mon, Jul 28, 2014 at 1:01 PM, Manman Ren <manman.ren at gmail.com> wrote:
> >
> >
> >
> > On Mon, Jul 28, 2014 at 12:40 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>
> >> On Mon, Jul 28, 2014 at 11:52 AM, Manman Ren <manman.ren at gmail.com>
> wrote:
> >> > Author: mren
> >> > Date: Mon Jul 28 13:52:30 2014
> >> > New Revision: 214111
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=214111&view=rev
> >> > Log:
> >> > [Debug Info] replace DIUnspecifiedParameter with DITrivialType.
> >> >
> >> > This is the first of a series of patches to handle type uniqueing of
> the
> >> > type array for a subroutine type.
> >> >
> >> > This commit makes sure unspecified_parameter is a DIType to enable
> >> > converting
> >> > the type array for a subroutine type to an array of DITypes.
> >>
> >> Can you remind me (& it'll be helpful to have it in this thread for
> >> later reference) why unspecified type couldn't be just a plain DIType
> >> (or DIBasicType, etc) and needed it's own derived class?
> >
> >
> > 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.
> > The other reason is for clarity so we know these two are trivial types
> > without size, offset etc.
> >
> > I am okay with removing DITrivialType if it is a burden to have one more
> > class in our DIType hierarchy.
>
> It really is a bit awkward to derive here - since unspecified type has
> strictly /less/ functionality (hence all the assertions that were
> added) - it should be further up the inheritance hierarchy, if
> anywhere. And it's not /exactly/ a type, though it's convenient to
> treat it as one.
>
Totally agree.
>
> We could be hackish in a different way, and treat null in the
> subroutine type list as being "unspecified type" (null in the first
> slot means "void return", but null in later (and only trailing)
> positions in the array could be "unspecified parameters")
>
> The "right" thing would be for there to be a type from which DIType
> would derive (call it, say, DIThingsInSubroutineTypes) that we either
> used directly, or had a separate derived type for
> DIUnspecifiedParameters - that type (DIThingsInSubroutineTypes)
> wouldn't have any of the member functions DIType has, because it
> doesn't have any data/features.
>
> But then we'd need DIThingsInSubroutineTypesRef as well, etc, etc...
>
> I'm sort of inclined to use null here. Eric - what do you reckon?
>
> {null} -> "void()"
> {null, null} -> "void(...)"
> {null, <metadata node for int>, null} -> "void(int, ...)"
> {null, null, <metadata node for int>} -> invalid
>
> Eric - what do you reckon?
>
> Easy to implement, just have createUnspecifiedParameter return null,
> and have the two places that check for isUnspecifiedParameter check
> for null instead.
>
> Any other reasons we'd have null in a parameter type list?
>
Sounds good to me. I can't think of any reason to have a null parameter
type.
>
> But, yeah, alternatively we just use a basic DIType and don't worry
> about the rest of the fields - it's a bit trickier I think (having a
> non-null DIType object with a bunch of operations that are invalid -
> null/non-nullness is a bit more of a clearer distinction - either
> you've got all the operations or none of them).
>
Just like unspecified_type, we can use zero or null for fields of
DIBasicType that are not needed.
Thanks,
Manman
> - David
>
> >
> > Thanks,
> > Manman
> >
> >>
> >>
> >> I'm guessing it's for size reasons, though I don't think it's too
> >> important to make unspecified_type particularly compact - we'll only
> >> ever need one of them anyway, so we just pay the cost once per module.
> >> We could still have the same assertions in DIType for usability,
> >> though (but they're probably not necessary either - many of DIType's
> >> members aren't valid/meaningful for various types).
> >>
> >> >
> >> > This commit should have no functionality change. With this commit, we
> >> > may
> >> > change unspecified type to be a DITrivialType instead of a DIType.
> >> >
> >> > Modified:
> >> > llvm/trunk/include/llvm/IR/DIBuilder.h
> >> > llvm/trunk/include/llvm/IR/DebugInfo.h
> >> > llvm/trunk/lib/IR/DIBuilder.cpp
> >> > llvm/trunk/lib/IR/DebugInfo.cpp
> >> >
> >> > Modified: llvm/trunk/include/llvm/IR/DIBuilder.h
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DIBuilder.h?rev=214111&r1=214110&r2=214111&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/include/llvm/IR/DIBuilder.h (original)
> >> > +++ llvm/trunk/include/llvm/IR/DIBuilder.h Mon Jul 28 13:52:30 2014
> >> > @@ -463,9 +463,9 @@ namespace llvm {
> >> > /// through debug info anchors.
> >> > void retainType(DIType T);
> >> >
> >> > - /// createUnspecifiedParameter - Create unspecified type
> descriptor
> >> > + /// createUnspecifiedParameter - Create unspecified parameter
> type
> >> > /// for a subroutine type.
> >> > - DIDescriptor createUnspecifiedParameter();
> >> > + DITrivialType createUnspecifiedParameter();
> >> >
> >> > /// getOrCreateArray - Get a DIArray, create one if required.
> >> > DIArray getOrCreateArray(ArrayRef<Value *> Elements);
> >> >
> >> > Modified: llvm/trunk/include/llvm/IR/DebugInfo.h
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfo.h?rev=214111&r1=214110&r2=214111&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/include/llvm/IR/DebugInfo.h (original)
> >> > +++ llvm/trunk/include/llvm/IR/DebugInfo.h Mon Jul 28 13:52:30 2014
> >> > @@ -128,6 +128,7 @@ public:
> >> > bool isDerivedType() const;
> >> > bool isCompositeType() const;
> >> > bool isBasicType() const;
> >> > + bool isTrivialType() const;
> >> > bool isVariable() const;
> >> > bool isSubprogram() const;
> >> > bool isGlobalVariable() const;
> >> > @@ -302,15 +303,36 @@ public:
> >> > /// Verify - Verify that a type descriptor is well formed.
> >> > bool Verify() const;
> >> >
> >> > - DIScopeRef getContext() const { return getFieldAs<DIScopeRef>(2); }
> >> > - StringRef getName() const { return getStringField(3); }
> >> > - unsigned getLineNumber() const { return getUnsignedField(4); }
> >> > - uint64_t getSizeInBits() const { return getUInt64Field(5); }
> >> > - uint64_t getAlignInBits() const { return getUInt64Field(6); }
> >> > + DIScopeRef getContext() const {
> >> > + assert(!isTrivialType() && "no context for DITrivialType");
> >> > + return getFieldAs<DIScopeRef>(2);
> >> > + }
> >> > + StringRef getName() const {
> >> > + assert(!isTrivialType() && "no name for DITrivialType");
> >> > + return getStringField(3);
> >> > + }
> >> > + unsigned getLineNumber() const {
> >> > + assert(!isTrivialType() && "no line number for DITrivialType");
> >> > + return getUnsignedField(4);
> >> > + }
> >> > + uint64_t getSizeInBits() const {
> >> > + assert(!isTrivialType() && "no SizeInBits for DITrivialType");
> >> > + return getUInt64Field(5);
> >> > + }
> >> > + uint64_t getAlignInBits() const {
> >> > + assert(!isTrivialType() && "no AlignInBits for DITrivialType");
> >> > + return getUInt64Field(6);
> >> > + }
> >> > // FIXME: Offset is only used for DW_TAG_member nodes. Making
> every
> >> > type
> >> > // carry this is just plain insane.
> >> > - uint64_t getOffsetInBits() const { return getUInt64Field(7); }
> >> > - unsigned getFlags() const { return getUnsignedField(8); }
> >> > + uint64_t getOffsetInBits() const {
> >> > + assert(!isTrivialType() && "no OffsetInBits for DITrivialType");
> >> > + return getUInt64Field(7);
> >> > + }
> >> > + unsigned getFlags() const {
> >> > + assert(!isTrivialType() && "no flag for DITrivialType");
> >> > + return getUnsignedField(8);
> >> > + }
> >> > bool isPrivate() const { return (getFlags() & FlagPrivate) != 0; }
> >> > bool isProtected() const { return (getFlags() & FlagProtected) !=
> 0;
> >> > }
> >> > bool isForwardDecl() const { return (getFlags() & FlagFwdDecl) !=
> 0;
> >> > }
> >> > @@ -343,6 +365,12 @@ public:
> >> > void replaceAllUsesWith(MDNode *D);
> >> > };
> >> >
> >> > +class DITrivialType : public DIType {
> >> > +public:
> >> > + explicit DITrivialType(const MDNode *N = nullptr) : DIType(N) {}
> >> > + bool Verify() const;
> >> > +};
> >> > +
> >> > /// DIBasicType - A basic type, like 'int' or 'float'.
> >> > class DIBasicType : public DIType {
> >> > public:
> >> > @@ -571,14 +599,6 @@ public:
> >> > bool Verify() const;
> >> > };
> >> >
> >> > -/// DIUnspecifiedParameter - This is a wrapper for unspecified
> >> > parameters.
> >> > -class DIUnspecifiedParameter : public DIDescriptor {
> >> > -public:
> >> > - explicit DIUnspecifiedParameter(const MDNode *N = nullptr)
> >> > - : DIDescriptor(N) {}
> >> > - bool Verify() const;
> >> > -};
> >> > -
> >> > /// DITemplateTypeParameter - This is a wrapper for template type
> >> > parameter.
> >> > class DITemplateTypeParameter : public DIDescriptor {
> >> > public:
> >> >
> >> > Modified: llvm/trunk/lib/IR/DIBuilder.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=214111&r1=214110&r2=214111&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/lib/IR/DIBuilder.cpp (original)
> >> > +++ llvm/trunk/lib/IR/DIBuilder.cpp Mon Jul 28 13:52:30 2014
> >> > @@ -875,11 +875,11 @@ void DIBuilder::retainType(DIType T) {
> >> >
> >> > /// createUnspecifiedParameter - Create unspeicified type descriptor
> >> > /// for the subroutine type.
> >> > -DIDescriptor DIBuilder::createUnspecifiedParameter() {
> >> > +DITrivialType DIBuilder::createUnspecifiedParameter() {
> >> > Value *Elts[] = {
> >> > GetTagConstant(VMContext, dwarf::DW_TAG_unspecified_parameters)
> >> > };
> >> > - return DIDescriptor(MDNode::get(VMContext, Elts));
> >> > + return DITrivialType(MDNode::get(VMContext, Elts));
> >> > }
> >> >
> >> > /// createForwardDecl - Create a temporary forward-declared type that
> >> >
> >> > Modified: llvm/trunk/lib/IR/DebugInfo.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=214111&r1=214110&r2=214111&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/lib/IR/DebugInfo.cpp (original)
> >> > +++ llvm/trunk/lib/IR/DebugInfo.cpp Mon Jul 28 13:52:30 2014
> >> > @@ -39,6 +39,7 @@ bool DIDescriptor::Verify() const {
> >> > return DbgNode &&
> >> > (DIDerivedType(DbgNode).Verify() ||
> >> > DICompositeType(DbgNode).Verify() ||
> >> > DIBasicType(DbgNode).Verify() ||
> >> > + DITrivialType(DbgNode).Verify() ||
> >> > DIVariable(DbgNode).Verify() ||
> >> > DISubprogram(DbgNode).Verify() ||
> >> > DIGlobalVariable(DbgNode).Verify() ||
> >> > DIFile(DbgNode).Verify() ||
> >> > DICompileUnit(DbgNode).Verify() ||
> >> > DINameSpace(DbgNode).Verify() ||
> >> > @@ -46,7 +47,6 @@ bool DIDescriptor::Verify() const {
> >> > DILexicalBlockFile(DbgNode).Verify() ||
> >> > DISubrange(DbgNode).Verify() ||
> >> > DIEnumerator(DbgNode).Verify() ||
> >> > DIObjCProperty(DbgNode).Verify() ||
> >> > - DIUnspecifiedParameter(DbgNode).Verify() ||
> >> > DITemplateTypeParameter(DbgNode).Verify() ||
> >> > DITemplateValueParameter(DbgNode).Verify() ||
> >> > DIImportedEntity(DbgNode).Verify());
> >> > @@ -155,6 +155,10 @@ MDNode *DIVariable::getInlinedAt() const
> >> > // Predicates
> >> >
> >> >
> //===----------------------------------------------------------------------===//
> >> >
> >> > +bool DIDescriptor::isTrivialType() const {
> >> > + return DbgNode && getTag() == dwarf::DW_TAG_unspecified_parameters;
> >> > +}
> >> > +
> >> > /// isBasicType - Return true if the specified tag is legal for
> >> > /// DIBasicType.
> >> > bool DIDescriptor::isBasicType() const {
> >> > @@ -225,7 +229,8 @@ bool DIDescriptor::isVariable() const {
> >> >
> >> > /// isType - Return true if the specified tag is legal for DIType.
> >> > bool DIDescriptor::isType() const {
> >> > - return isBasicType() || isCompositeType() || isDerivedType();
> >> > + return isBasicType() || isCompositeType() || isDerivedType() ||
> >> > + isTrivialType();
> >> > }
> >> >
> >> > /// isSubprogram - Return true if the specified tag is legal for
> >> > @@ -456,7 +461,7 @@ bool DIType::Verify() const {
> >> >
> >> > // FIXME: Sink this into the various subclass verifies.
> >> > uint16_t Tag = getTag();
> >> > - if (!isBasicType() && Tag != dwarf::DW_TAG_const_type &&
> >> > + if (!isBasicType() && !isTrivialType() && Tag !=
> >> > dwarf::DW_TAG_const_type &&
> >> > Tag != dwarf::DW_TAG_volatile_type && Tag !=
> >> > dwarf::DW_TAG_pointer_type &&
> >> > Tag != dwarf::DW_TAG_ptr_to_member_type &&
> >> > Tag != dwarf::DW_TAG_reference_type &&
> >> > @@ -471,6 +476,8 @@ bool DIType::Verify() const {
> >> > // a CompositeType.
> >> > if (isBasicType())
> >> > return DIBasicType(DbgNode).Verify();
> >> > + else if (isTrivialType())
> >> > + return DITrivialType(DbgNode).Verify();
> >> > else if (isCompositeType())
> >> > return DICompositeType(DbgNode).Verify();
> >> > else if (isDerivedType())
> >> > @@ -484,6 +491,10 @@ bool DIBasicType::Verify() const {
> >> > return isBasicType() && DbgNode->getNumOperands() == 10;
> >> > }
> >> >
> >> > +bool DITrivialType::Verify() const {
> >> > + return isTrivialType() && DbgNode->getNumOperands() == 1;
> >> > +}
> >> > +
> >> > /// Verify - Verify that a derived type descriptor is well formed.
> >> > bool DIDerivedType::Verify() const {
> >> > // Make sure DerivedFrom @ field 9 is TypeRef.
> >> > @@ -624,11 +635,6 @@ bool DILexicalBlockFile::Verify() const
> >> > return isLexicalBlockFile() && DbgNode->getNumOperands() == 3;
> >> > }
> >> >
> >> > -/// \brief Verify that an unspecified parameter descriptor is well
> >> > formed.
> >> > -bool DIUnspecifiedParameter::Verify() const {
> >> > - return isUnspecifiedParameter() && DbgNode->getNumOperands() == 1;
> >> > -}
> >> > -
> >> > /// \brief Verify that the template type parameter descriptor is well
> >> > formed.
> >> > bool DITemplateTypeParameter::Verify() const {
> >> > return isTemplateTypeParameter() && DbgNode->getNumOperands() == 7;
> >> > @@ -1290,7 +1296,7 @@ void DIEnumerator::printInternal(raw_ost
> >> > }
> >> >
> >> > void DIType::printInternal(raw_ostream &OS) const {
> >> > - if (!DbgNode)
> >> > + if (!DbgNode || isTrivialType())
> >> > return;
> >> >
> >> > StringRef Res = getName();
> >> >
> >> >
> >> > _______________________________________________
> >> > 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/20140728/a0def1b2/attachment.html>
More information about the llvm-commits
mailing list