[llvm] r190418 - Debug Info: define a DIRef template.
David Blaikie
dblaikie at gmail.com
Tue Sep 10 16:28:34 PDT 2013
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.
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/20130910/c2c79725/attachment.html>
More information about the llvm-commits
mailing list