[llvm] r192018 - Debug Info: In DIBuilder, the derived-from field of a DW_TAG_pointer_type

[Manman Ren]

On Mon, Oct 7, 2013 at 2:43 PM, Eric Christopher <echristo at gmail.com> wrote:

> > +  /// If the scope node has a name, return that, else return an empty
> string.
> > +  StringRef getName() const;
> >    StringRef getFilename() const;
> >    StringRef getDirectory() const;
> >
> > @@ -227,6 +230,16 @@ template <typename T> class DIRef {
> >
> >  public:
> >    T resolve(const DITypeIdentifierMap &Map) const;
> > +  StringRef getName() const {
> > +    if (!Val)
> > +      return StringRef();
> > +
> > +    if (const MDNode *MD = dyn_cast<MDNode>(Val))
> > +      return T(MD).getName();
> > +
> > +    const MDString *MS = cast<MDString>(Val);
> > +    return MS->getString();
> > +  }
>
> Please take this out of line.
>
Done.

>
> >    operator Value *() const { return const_cast<Value *>(Val); }
> >  };
> >
> > @@ -300,9 +313,6 @@ public:
> >    bool isStaticMember() const { return (getFlags() & FlagStaticMember)
> != 0; }
> >    bool isValid() const { return DbgNode && isType(); }
> >
> > -  /// isUnsignedDIType - Return true if type encoding is unsigned.
> > -  bool isUnsignedDIType();
> > -
> >    /// replaceAllUsesWith - Replace all uses of debug info referenced by
> >    /// this descriptor.
> >    void replaceAllUsesWith(DIDescriptor &D);
> > @@ -330,11 +340,7 @@ class DIDerivedType : public DIType {
> >  public:
> >    explicit DIDerivedType(const MDNode *N = 0) : DIType(N) {}
> >
> > -  DIType getTypeDerivedFrom() const { return getFieldAs<DIType>(9); }
> > -
> > -  /// getOriginalTypeSize - If this type is derived from a base type
> then
> > -  /// return base type size.
> > -  uint64_t getOriginalTypeSize() const;
> > +  DITypeRef getTypeDerivedFrom() const { return
> getFieldAs<DITypeRef>(9); }
> >
> >    /// getObjCProperty - Return property node, if this ivar is
> >    /// associated with one.
> >
> > Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=192018&r1=192017&r2=192018&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp Fri Oct  4
> 20:43:03 2013
> > @@ -520,7 +520,7 @@ void CompileUnit::addBlockByrefAddress(c
> >
> >    if (Tag == dwarf::DW_TAG_pointer_type) {
> >      DIDerivedType DTy = DIDerivedType(Ty);
> > -    TmpTy = DTy.getTypeDerivedFrom();
> > +    TmpTy = DD->resolve(DTy.getTypeDerivedFrom());
>
> Please use the accessor I added.
>
Done

>
> >      isPointer = true;
> >    }
> >
> > @@ -587,9 +587,10 @@ void CompileUnit::addBlockByrefAddress(c
> >  }
> >
> >  /// isTypeSigned - Return true if the type is signed.
> > -static bool isTypeSigned(DIType Ty, int *SizeInBits) {
> > +static bool isTypeSigned(DwarfDebug *DD, DIType Ty, int *SizeInBits) {
> >    if (Ty.isDerivedType())
> > -    return isTypeSigned(DIDerivedType(Ty).getTypeDerivedFrom(),
> SizeInBits);
> > +    return isTypeSigned(DD,
> DD->resolve(DIDerivedType(Ty).getTypeDerivedFrom()),
> > +                        SizeInBits);
>
> Again.
>
Done

>
> >    if (Ty.isBasicType())
> >      if (DIBasicType(Ty).getEncoding() == dwarf::DW_ATE_signed
> >          || DIBasicType(Ty).getEncoding() == dwarf::DW_ATE_signed_char) {
> > @@ -599,6 +600,51 @@ static bool isTypeSigned(DIType Ty, int
> >    return false;
> >  }
> >
> > +/// Return true if type encoding is unsigned.
> > +static bool isUnsignedDIType(DwarfDebug *DD, DIType Ty) {
> > +  DIDerivedType DTy(Ty);
> > +  if (DTy.isDerivedType())
> > +    return isUnsignedDIType(DD, DD->resolve(DTy.getTypeDerivedFrom()));
> > +
>
> Again, and everywhere else, please make sure you get them all.
>
Done

>
> > +  DIBasicType BTy(Ty);
> > +  if (BTy.isBasicType()) {
> > +    unsigned Encoding = BTy.getEncoding();
> > +    if (Encoding == dwarf::DW_ATE_unsigned ||
> > +        Encoding == dwarf::DW_ATE_unsigned_char ||
> > +        Encoding == dwarf::DW_ATE_boolean)
> > +      return true;
> > +  }
> > +  return false;
> > +}
> > +
> > +/// If this type is derived from a base type then return base type size.
> > +static uint64_t getOriginalTypeSize(DwarfDebug *DD, DIDerivedType Ty) {
>
> Since we're moving and changing this one, how about getBaseTypeSize?
>
Done

>
> Also we might want to just make this a method on DwarfDebug since it
> has the information to connect the various references.
>

We have a few static helper functions: isTypeSigned, getBaseTypeSize, and
isUnsignedDIType.
isTypeUnitScoped traces along the context chain while the above three
traces along the derived-from chain.
Should we make all four member functions then? DwarfDebug or CompileUnit?

I will rename isUnsignedDIType to isTypeUnsigned.


> > -  bool SignedConstant = isTypeSigned(Ty, &SizeInBits);
> > +  bool SignedConstant = isTypeSigned(DD, Ty, &SizeInBits);
>
> Same with isTypeSigned.
>
See above.

>
>
> > Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=192018&r1=192017&r2=192018&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Fri Oct  4 20:43:03
> 2013
> > @@ -149,13 +149,13 @@ DIType DbgVariable::getType() const {
> >      uint16_t tag = Ty.getTag();
> >
> >      if (tag == dwarf::DW_TAG_pointer_type)
> > -      subType = DIDerivedType(Ty).getTypeDerivedFrom();
> > +      subType = DD->resolve(DIDerivedType(Ty).getTypeDerivedFrom());
> >
> >      DIArray Elements = DICompositeType(subType).getTypeArray();
> >      for (unsigned i = 0, N = Elements.getNumElements(); i < N; ++i) {
> >        DIDerivedType DT = DIDerivedType(Elements.getElement(i));
> >        if (getName() == DT.getName())
> > -        return (DT.getTypeDerivedFrom());
> > +        return (DD->resolve(DT.getTypeDerivedFrom()));
>
> Please add a resolve function to DbgVariable so that the interface is
> the same here as well.
>
Done.


>
> This part of the change could have been done separately and would have
> made reviewing cleaner and easier.
>
> > +// If the scope node has a name, return that, else return an empty
> string.
> > +StringRef DIScope::getName() const {
> > +  if (isType())
> > +    return DIType(DbgNode).getName();
> > +  if (isSubprogram())
> > +    return DISubprogram(DbgNode).getName();
> > +  if (isNameSpace())
> > +    return DINameSpace(DbgNode).getName();
> > +  assert((isLexicalBlock() || isLexicalBlockFile() || isFile() ||
> > +          isCompileUnit()) && "Unhandled type of scope.");
> > +  return StringRef();
> > +}
> > +
>
> This could have been broken out separately as well.
>
> Please also construct the small testcase that accurately tests the
> changes you made without relying the linking of two modules.
>
Incoming.

Manman

>
> Thanks.
>
> -eric
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131008/72698485/attachment.html>