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

[Eric Christopher]

> +  /// 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.

>    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.

>      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.

>    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.

> +  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?

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

> -  bool SignedConstant = isTypeSigned(Ty, &SizeInBits);
> +  bool SignedConstant = isTypeSigned(DD, Ty, &SizeInBits);

Same with isTypeSigned.


> 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.

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.

Thanks.

-eric