[llvm] r190418 - Debug Info: define a DIRef template.
Manman Ren
manman.ren at gmail.com
Wed Sep 11 12:15:15 PDT 2013
On Tue, Sep 10, 2013 at 5:38 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Tue, Sep 10, 2013 at 5:24 PM, Manman Ren <manman.ren at gmail.com> wrote:
>
>>
>>
>>
>> On Tue, Sep 10, 2013 at 4:28 PM, David Blaikie <dblaikie at gmail.com>wrote:
>>
>>>
>>>
>>>
>>> On Tue, Sep 10, 2013 at 3:31 PM, Manman Ren <manman.ren at gmail.com>wrote:
>>>
>>>>
>>>>
>>>>
>>>> On Tue, Sep 10, 2013 at 12:54 PM, David Blaikie <dblaikie at gmail.com>wrote:
>>>>
>>>>> I assume the specializations I've asked about are there because you
>>>>> didn't have a good way to generalize over scopes and types. What sort of
>>>>> solutions did you consider to avoid this duplication?
>>>>>
>>>> By duplication, do you mean the template?
>>>>
>>>
>>> The explicit specializations of the
>>>
>>>
>>>>
>>>>> On Sep 10, 2013 11:35 AM, "Manman Ren" <manman.ren at gmail.com> wrote:
>>>>> >
>>>>> > Author: mren
>>>>> > Date: Tue Sep 10 13:30:07 2013
>>>>> > New Revision: 190418
>>>>> >
>>>>> > URL: http://llvm.org/viewvc/llvm-project?rev=190418&view=rev
>>>>> > Log:
>>>>> > Debug Info: define a DIRef template.
>>>>> >
>>>>> > Specialize the constructors for DIRef<DIScope> and DIRef<DIType> to
>>>>> make sure
>>>>> > the Value is indeed a scope ref and a type ref.
>>>>> >
>>>>> > Use DIScopeRef for DIScope::getContext and DIType::getContext and
>>>>> use DITypeRef
>>>>> > for getContainingType and getClassType.
>>>>> >
>>>>> > DIScope::generateRef now returns a DIScopeRef instead of a "Value *"
>>>>> for
>>>>> > readability and type safety.
>>>>> >
>>>>> > Modified:
>>>>> > llvm/trunk/include/llvm/DebugInfo.h
>>>>> > llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>>>> > llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>>>> > llvm/trunk/lib/IR/DebugInfo.cpp
>>>>> >
>>>>> > Modified: llvm/trunk/include/llvm/DebugInfo.h
>>>>> > URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo.h?rev=190418&r1=190417&r2=190418&view=diff
>>>>> >
>>>>> ==============================================================================
>>>>> > --- llvm/trunk/include/llvm/DebugInfo.h (original)
>>>>> > +++ llvm/trunk/include/llvm/DebugInfo.h Tue Sep 10 13:30:07 2013
>>>>> > @@ -17,6 +17,7 @@
>>>>> > #ifndef LLVM_DEBUGINFO_H
>>>>> > #define LLVM_DEBUGINFO_H
>>>>> >
>>>>> > +#include "llvm/Support/Casting.h"
>>>>> > #include "llvm/ADT/DenseMap.h"
>>>>> > #include "llvm/ADT/SmallPtrSet.h"
>>>>> > #include "llvm/ADT/SmallVector.h"
>>>>> > @@ -46,7 +47,7 @@ namespace llvm {
>>>>> > class DILexicalBlockFile;
>>>>> > class DIVariable;
>>>>> > class DIType;
>>>>> > - class DIScopeRef;
>>>>> > + class DIScope;
>>>>> > class DIObjCProperty;
>>>>> >
>>>>> > /// Maps from type identifier to the actual MDNode.
>>>>> > @@ -56,9 +57,10 @@ namespace llvm {
>>>>> > /// This should not be stored in a container, because the
>>>>> underlying MDNode
>>>>> > /// may change in certain situations.
>>>>> > class DIDescriptor {
>>>>> > - // Befriends DIScopeRef so DIScopeRef can befriend the
>>>>> protected member
>>>>> > - // function: getFieldAs<DIScopeRef>.
>>>>> > - friend class DIScopeRef;
>>>>> > + // Befriends DIRef so DIRef can befriend the protected member
>>>>> > + // function: getFieldAs<DIRef>.
>>>>> > + template <typename T>
>>>>> > + friend class DIRef;
>>>>> > public:
>>>>> > enum {
>>>>> > FlagPrivate = 1 << 0,
>>>>> > @@ -151,10 +153,6 @@ namespace llvm {
>>>>> > void dump() const;
>>>>> > };
>>>>> >
>>>>> > - /// npecialize getFieldAs to handle fields that are references to
>>>>> DIScopes.
>>>>> > - template <>
>>>>> > - DIScopeRef DIDescriptor::getFieldAs<DIScopeRef>(unsigned Elt)
>>>>> const;
>>>>> > -
>>>>> > /// DISubrange - This is used to represent ranges, for array
>>>>> bounds.
>>>>> > class DISubrange : public DIDescriptor {
>>>>> > friend class DIDescriptor;
>>>>> > @@ -192,6 +190,10 @@ namespace llvm {
>>>>> > bool Verify() const;
>>>>> > };
>>>>> >
>>>>> > + template <typename T>
>>>>> > + class DIRef;
>>>>> > + typedef DIRef<DIScope> DIScopeRef;
>>>>>
>>>>> Where's the corresponding dityperef? Could we just declare it here for
>>>>> consistency?
>>>>>
>>>> Sure, I can move it here. Right now, DITypeRef is after def of DIType.
>>>>
>>>
>>> Given we already have a forward declaration of DIType up the top, we
>>> could move the typedef up here I think.
>>>
>>>
>>>> > +
>>>>> > /// DIScope - A base class for various scopes.
>>>>> > class DIScope : public DIDescriptor {
>>>>> > protected:
>>>>> > @@ -208,23 +210,7 @@ namespace llvm {
>>>>> >
>>>>> > /// Generate a reference to this DIScope. Uses the type
>>>>> identifier instead
>>>>> > /// of the actual MDNode if possible, to help type uniquing.
>>>>> > - Value *generateRef();
>>>>> > - };
>>>>> > -
>>>>> > - /// Represents reference to a DIScope, abstracts over direct and
>>>>> > - /// identifier-based metadata scope references.
>>>>> > - class DIScopeRef {
>>>>> > - template <typename DescTy>
>>>>> > - friend DescTy DIDescriptor::getFieldAs(unsigned Elt) const;
>>>>> > - friend DIScopeRef DIScope::getContext() const;
>>>>> > -
>>>>> > - /// Val can be either a MDNode or a MDString, in the latter,
>>>>> > - /// MDString specifies the type identifier.
>>>>> > - const Value *Val;
>>>>> > - explicit DIScopeRef(const Value *V);
>>>>> > - public:
>>>>> > - DIScope resolve(const DITypeIdentifierMap &Map) const;
>>>>> > - operator Value *() const { return const_cast<Value*>(Val); }
>>>>> > + DIScopeRef generateRef();
>>>>> > };
>>>>> >
>>>>> > /// DIType - This is a wrapper for a type.
>>>>> > @@ -241,7 +227,7 @@ namespace llvm {
>>>>> > /// Verify - Verify that a type descriptor is well formed.
>>>>> > bool Verify() const;
>>>>> >
>>>>> > - DIScopeRef getContext() const { return
>>>>> getFieldAs<DIScopeRef>(2); }
>>>>> > + DIScopeRef getContext() const;
>>>>>
>>>>> Why did this become our of line?
>>>>>
>>>> Because of dependency issues,
>>>>
>>>
>>> What's the dependency cycle?
>>>
>>>
>>>> we can't construct a DIScopeRef here without a full definition.
>>>> And the full def of DIRef needs to be after the full def of DIType
>>>> since we now have the def of DIRef::resolve in the header file.
>>>>
>>>
>>> This seems a bit strange, since DIRef shouldn't be specific to type or
>>> scope, ideally.
>>>
>>
>> It was the assertion in resolve:
>>
>> assert(DIType(Iter->second).isType() &&
>> > + "MDNode in DITypeIdentifierMap should be a DIType.");
>>
>> which requires the full def of DIType to compile.
>> But we should be able to change from DIType().isType() to
>> DIDescriptor().isType().
>>
>
> Right
>
>
>> Then the def of DIRef will only need to be after DIScope because of
>> > + friend DIScopeRef DIScope::getContext() const;
>> > + friend DIScopeRef DIScope::generateRef();
>>
>
> Why would it need to be after those? It'll be calling them on a dependent
> expression of type T, rather than type DIScope, right? So the template can
> appear before the definition of DIScope.
>
Changing from DIScope::generateRef to T::generateRef means we need an
implementation of DIType::generateRef.
For every T, we require an implementation of T::generateRef and
T::getContext.
>
> Also - I'm still a bit dubious of "generateRef" - it should at least be
> const & I still think "getRef" might make more sense.
>
Yes, we can constify generateRef. I am okay with changing it to getRef, but
the other get functions usually mean accessing a field.
Manman
> Or we could make a public (explicit) ctor for DIRef<T> from const T&? But
> I suppose that'd be different between a DIScope and a DIType, etc... so
> that's not great.
>
>
>>
>>
>> Manman
>>
>>
>>>
>>> So I assume the reason that DIRef is specific to type is for use of
>>> DIDescriptor::isType rather than isScope. Is there some way we could
>>> generalize over these based on the template parameter to DIRef?
>>>
>>> Eric - this is a question for you too. If the "isFoo" functions were
>>> instead members of the relevant DI* functions ("isValid" or is that too
>>> subtly different to "Verify"?) then we could use "bool T::is()" (bikeshed
>>> as appropriate) in this template.
>>>
>>> Of course that would mean rewriting existing calls to x.isFoo()
>>> functions into x.is() or Foo(x).is(), or keeping the duplicates (which
>>> would seem a bit unfortunate).
>>>
>>>
>>>> Moving resolve to the header file is to avoid a link error.
>>>>
>>> > StringRef getName() const { return getStringField(3);
>>>>> }
>>>>> > unsigned getLineNumber() const { return
>>>>> getUnsignedField(4); }
>>>>> > uint64_t getSizeInBits() const { return getUInt64Field(5);
>>>>> }
>>>>> > @@ -297,6 +283,53 @@ namespace llvm {
>>>>> > void replaceAllUsesWith(MDNode *D);
>>>>> > };
>>>>> >
>>>>> > + /// Represents reference to a DIDescriptor, abstracts over direct
>>>>> and
>>>>> > + /// identifier-based metadata references.
>>>>> > + template <typename T>
>>>>> > + class DIRef {
>>>>> > + template <typename DescTy>
>>>>> > + friend DescTy DIDescriptor::getFieldAs(unsigned Elt) const;
>>>>> > + friend DIScopeRef DIScope::getContext() const;
>>>>> > + friend DIScopeRef DIScope::generateRef();
>>>>> > +
>>>>> > + /// Val can be either a MDNode or a MDString, in the latter,
>>>>> > + /// MDString specifies the type identifier.
>>>>> > + const Value *Val;
>>>>> > + explicit DIRef(const Value *V);
>>>>> > + public:
>>>>> > + T resolve(const DITypeIdentifierMap &Map) const {
>>>>> > + if (!Val)
>>>>> > + return T();
>>>>> > +
>>>>> > + if (const MDNode *MD = dyn_cast<MDNode>(Val))
>>>>> > + return T(MD);
>>>>> > +
>>>>> > + const MDString *MS = cast<MDString>(Val);
>>>>> > + // Find the corresponding MDNode.
>>>>> > + DITypeIdentifierMap::const_iterator Iter = Map.find(MS);
>>>>> > + assert(Iter != Map.end() && "Identifier not in the type
>>>>> map?");
>>>>> > + assert(DIType(Iter->second).isType() &&
>>>>> > + "MDNode in DITypeIdentifierMap should be a DIType.");
>>>>>
>>>>> Could we write this generically somehow? This looks like a
>>>>> type-secific (doesn't work for scopes) implementation.
>>>>>
>>>> The MDNode found in the TypeIdentifierMap should be a DIType, right?
>>>>
>>>>> > + return T(Iter->second);
>>>>> > + }
>>>>> > + operator Value *() const { return const_cast<Value*>(Val); }
>>>>> > + };
>>>>> > +
>>>>> > + /// Specialize getFieldAs to handle fields that are references to
>>>>> DIScopes.
>>>>> > + template <>
>>>>> > + DIScopeRef DIDescriptor::getFieldAs<DIScopeRef>(unsigned Elt)
>>>>> const;
>>>>> > + /// Specialize DIRef constructor for DIScopeRef.
>>>>>
>>>>> Why?
>>>>>
>>>> Just to have more specific checking of the passed-in Value: use
>>>> isScopeRef in the constructor.
>>>>
>>>>> > + template <>
>>>>> > + DIRef<DIScope>::DIRef(const Value *V);
>>>>> > +
>>>>> > + typedef DIRef<DIType> DITypeRef;
>>>>> > + /// Specialize getFieldAs to handle fields that are references to
>>>>> DITypes.
>>>>> > + template <>
>>>>> > + DITypeRef DIDescriptor::getFieldAs<DITypeRef>(unsigned Elt) const;
>>>>> > + /// Specialize DIRef constructor for DITypeRef.
>>>>> > + template <>
>>>>> > + DIRef<DIType>::DIRef(const Value *V);
>>>>>
>>>>> And this ?
>>>>>
>>>> To have more specific checking of the passed-in Value: use isTypeRef in
>>>> the constructor.
>>>>
>>>> Manman
>>>>
>>>>> > +
>>>>> > /// DIBasicType - A basic type, like 'int' or 'float'.
>>>>> > class DIBasicType : public DIType {
>>>>> > public:
>>>>> > @@ -328,9 +361,9 @@ namespace llvm {
>>>>> > /// associated with one.
>>>>> > MDNode *getObjCProperty() const;
>>>>> >
>>>>> > - DIScopeRef getClassType() const {
>>>>> > + DITypeRef getClassType() const {
>>>>> > assert(getTag() == dwarf::DW_TAG_ptr_to_member_type);
>>>>> > - return getFieldAs<DIScopeRef>(10);
>>>>> > + return getFieldAs<DITypeRef>(10);
>>>>> > }
>>>>> >
>>>>> > Constant *getConstant() const {
>>>>> > @@ -358,8 +391,8 @@ namespace llvm {
>>>>> > void setTypeArray(DIArray Elements, DIArray TParams =
>>>>> DIArray());
>>>>> > void addMember(DIDescriptor D);
>>>>> > unsigned getRunTimeLang() const { return getUnsignedField(11); }
>>>>> > - DIScopeRef getContainingType() const {
>>>>> > - return getFieldAs<DIScopeRef>(12);
>>>>> > + DITypeRef getContainingType() const {
>>>>> > + return getFieldAs<DITypeRef>(12);
>>>>> > }
>>>>> > void setContainingType(DICompositeType ContainingType);
>>>>> > DIArray getTemplateParams() const { return
>>>>> getFieldAs<DIArray>(13); }
>>>>> > @@ -426,8 +459,8 @@ namespace llvm {
>>>>> > unsigned getVirtuality() const { return getUnsignedField(10); }
>>>>> > unsigned getVirtualIndex() const { return getUnsignedField(11);
>>>>> }
>>>>> >
>>>>> > - DIScopeRef getContainingType() const {
>>>>> > - return getFieldAs<DIScopeRef>(12);
>>>>> > + DITypeRef getContainingType() const {
>>>>> > + return getFieldAs<DITypeRef>(12);
>>>>> > }
>>>>> >
>>>>> > unsigned getFlags() const {
>>>>> >
>>>>> > Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>>>> > URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=190418&r1=190417&r2=190418&view=diff
>>>>> >
>>>>> ==============================================================================
>>>>> > --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>>>>> > +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Tue Sep 10
>>>>> 13:30:07 2013
>>>>> > @@ -2644,8 +2644,3 @@ void DwarfDebug::emitDebugStrDWO() {
>>>>> >
>>>>> InfoHolder.emitStrings(Asm->getObjFileLowering().getDwarfStrDWOSection(),
>>>>> > OffSec, StrSym);
>>>>> > }
>>>>> > -
>>>>> > -/// Find the MDNode for the given scope reference.
>>>>> > -DIScope DwarfDebug::resolve(DIScopeRef SRef) const {
>>>>> > - return SRef.resolve(TypeIdentifierMap);
>>>>> > -}
>>>>> >
>>>>> > Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>>>> > URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=190418&r1=190417&r2=190418&view=diff
>>>>> >
>>>>> ==============================================================================
>>>>> > --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
>>>>> > +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Tue Sep 10
>>>>> 13:30:07 2013
>>>>> > @@ -684,7 +684,10 @@ public:
>>>>> > unsigned getDwarfVersion() const { return DwarfVersion; }
>>>>> >
>>>>> > /// Find the MDNode for the given scope reference.
>>>>> > - DIScope resolve(DIScopeRef SRef) const;
>>>>> > + template <typename T>
>>>>> > + T resolve(DIRef<T> Ref) const {
>>>>> > + return Ref.resolve(TypeIdentifierMap);
>>>>> > + }
>>>>> >
>>>>> > /// isSubprogramContext - Return true if Context is either a
>>>>> subprogram
>>>>> > /// or another context nested inside a subprogram.
>>>>> >
>>>>> > Modified: llvm/trunk/lib/IR/DebugInfo.cpp
>>>>> > URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=190418&r1=190417&r2=190418&view=diff
>>>>> >
>>>>> ==============================================================================
>>>>> > --- llvm/trunk/lib/IR/DebugInfo.cpp (original)
>>>>> > +++ llvm/trunk/lib/IR/DebugInfo.cpp Tue Sep 10 13:30:07 2013
>>>>> > @@ -725,13 +725,13 @@ void DICompositeType::addMember(DIDescri
>>>>> >
>>>>> > /// Generate a reference to this DIType. Uses the type identifier
>>>>> instead
>>>>> > /// of the actual MDNode if possible, to help type uniquing.
>>>>> > -Value *DIScope::generateRef() {
>>>>> > +DIScopeRef DIScope::generateRef() {
>>>>> > if (!isCompositeType())
>>>>> > - return *this;
>>>>> > + return DIScopeRef(*this);
>>>>> > DICompositeType DTy(DbgNode);
>>>>> > if (!DTy.getIdentifier())
>>>>> > - return *this;
>>>>> > - return DTy.getIdentifier();
>>>>> > + return DIScopeRef(*this);
>>>>> > + return DIScopeRef(DTy.getIdentifier());
>>>>> > }
>>>>> >
>>>>> > /// \brief Set the containing type.
>>>>> > @@ -1432,26 +1432,14 @@ void DIVariable::printExtendedName(raw_o
>>>>> > }
>>>>> > }
>>>>> >
>>>>> > -DIScopeRef::DIScopeRef(const Value *V) : Val(V) {
>>>>> > +/// Specialize constructor to make sure it has the correct type.
>>>>> > +template <>
>>>>> > +DIRef<DIScope>::DIRef(const Value *V) : Val(V) {
>>>>> > assert(isScopeRef(V) && "DIScopeRef should be a MDString or
>>>>> MDNode");
>>>>> > }
>>>>> > -
>>>>> > -/// Given a DITypeIdentifierMap, tries to find the corresponding
>>>>> > -/// DIScope for a DIScopeRef.
>>>>> > -DIScope DIScopeRef::resolve(const DITypeIdentifierMap &Map) const {
>>>>> > - if (!Val)
>>>>> > - return DIScope();
>>>>> > -
>>>>> > - if (const MDNode *MD = dyn_cast<MDNode>(Val))
>>>>> > - return DIScope(MD);
>>>>> > -
>>>>> > - const MDString *MS = cast<MDString>(Val);
>>>>> > - // Find the corresponding MDNode.
>>>>> > - DITypeIdentifierMap::const_iterator Iter = Map.find(MS);
>>>>> > - assert(Iter != Map.end() && "Identifier not in the type map?");
>>>>> > - assert(DIType(Iter->second).isType() &&
>>>>> > - "MDNode in DITypeIdentifierMap should be a DIType.");
>>>>> > - return DIScope(Iter->second);
>>>>> > +template <>
>>>>> > +DIRef<DIType>::DIRef(const Value *V) : Val(V) {
>>>>> > + assert(isTypeRef(V) && "DITypeRef should be a MDString or
>>>>> MDNode");
>>>>> > }
>>>>> >
>>>>> > /// Specialize getFieldAs to handle fields that are references to
>>>>> DIScopes.
>>>>> > @@ -1459,3 +1447,12 @@ template <>
>>>>> > DIScopeRef DIDescriptor::getFieldAs<DIScopeRef>(unsigned Elt) const
>>>>> {
>>>>> > return DIScopeRef(getField(DbgNode, Elt));
>>>>> > }
>>>>> > +/// Specialize getFieldAs to handle fields that are references to
>>>>> DITypes.
>>>>> > +template <>
>>>>> > +DITypeRef DIDescriptor::getFieldAs<DITypeRef>(unsigned Elt) const {
>>>>> > + return DITypeRef(getField(DbgNode, Elt));
>>>>> > +}
>>>>> > +
>>>>> > +DIScopeRef DIType::getContext() const {
>>>>> > + return getFieldAs<DIScopeRef>(2);
>>>>> > +}
>>>>> >
>>>>> >
>>>>> > _______________________________________________
>>>>> > 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/20130911/f56b3ce4/attachment.html>
More information about the llvm-commits
mailing list