[llvm] r214189 - [Debug Info] remove DITrivialType and use null to represent unspecified param.

David Blaikie dblaikie at gmail.com
Tue Jul 29 15:25:43 PDT 2014


On Tue, Jul 29, 2014 at 11:20 AM, Manman Ren <manman.ren at gmail.com> wrote:
> Author: mren
> Date: Tue Jul 29 13:20:39 2014
> New Revision: 214189
>
> URL: http://llvm.org/viewvc/llvm-project?rev=214189&view=rev
> Log:
> [Debug Info] remove DITrivialType and use null to represent unspecified param.
>
> Per feedback on r214111, we are going to use null to represent unspecified
> parameter. If the type array is {null}, it means a function that returns void;
> If the type array is {null, null}, it means a variadic function that returns
> void. In summary if we have more than one element in the type array and the last
> element is null, it is a variadic function.

For future reference, this sort of thing (where not much or nothing
from the previous patch is kept) is often easier to review by
reverting the previous patch and committing a new one.

>
> rdar://17628609
>
> Modified:
>     llvm/trunk/include/llvm/IR/DIBuilder.h
>     llvm/trunk/include/llvm/IR/DebugInfo.h
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>     llvm/trunk/lib/IR/DIBuilder.cpp
>     llvm/trunk/lib/IR/DebugInfo.cpp
>     llvm/trunk/test/DebugInfo/varargs.ll
>
> Modified: llvm/trunk/include/llvm/IR/DIBuilder.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DIBuilder.h?rev=214189&r1=214188&r2=214189&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/DIBuilder.h (original)
> +++ llvm/trunk/include/llvm/IR/DIBuilder.h Tue Jul 29 13:20:39 2014
> @@ -465,7 +465,7 @@ namespace llvm {
>
>      /// createUnspecifiedParameter - Create unspecified parameter type
>      /// for a subroutine type.
> -    DITrivialType createUnspecifiedParameter();
> +    DIBasicType 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=214189&r1=214188&r2=214189&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/DebugInfo.h (original)
> +++ llvm/trunk/include/llvm/IR/DebugInfo.h Tue Jul 29 13:20:39 2014
> @@ -129,7 +129,6 @@ public:
>    bool isCompositeType() const;
>    bool isSubroutineType() const;
>    bool isBasicType() const;
> -  bool isTrivialType() const;
>    bool isVariable() const;
>    bool isSubprogram() const;
>    bool isGlobalVariable() const;
> @@ -142,7 +141,6 @@ public:
>    bool isSubrange() const;
>    bool isEnumerator() const;
>    bool isType() const;
> -  bool isUnspecifiedParameter() const;
>    bool isTemplateTypeParameter() const;
>    bool isTemplateValueParameter() const;
>    bool isObjCProperty() const;
> @@ -308,36 +306,15 @@ public:
>    /// Verify - Verify that a type descriptor is well formed.
>    bool Verify() const;
>
> -  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);
> -  }
> +  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); }
>    // FIXME: Offset is only used for DW_TAG_member nodes.  Making every type
>    // carry this is just plain insane.
> -  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);
> -  }
> +  uint64_t getOffsetInBits() const { return getUInt64Field(7); }
> +  unsigned getFlags() const { return getUnsignedField(8); }
>    bool isPrivate() const { return (getFlags() & FlagPrivate) != 0; }
>    bool isProtected() const { return (getFlags() & FlagProtected) != 0; }
>    bool isForwardDecl() const { return (getFlags() & FlagFwdDecl) != 0; }
> @@ -370,12 +347,6 @@ 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:
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=214189&r1=214188&r2=214189&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Tue Jul 29 13:20:39 2014
> @@ -469,11 +469,13 @@ DIE *DwarfDebug::createScopeChildrenDIE(
>      // If this is a variadic function, add an unspecified parameter.
>      DISubprogram SP(Scope->getScopeNode());
>      DITypeArray FnArgs = SP.getType().getTypeArray();
> -    if (resolve(FnArgs.getElement(FnArgs.getNumElements() - 1))
> -            .isUnspecifiedParameter()) {
> +    // If we have a single element of null, it is a function that returns void.
> +    // If we have more than one elements and the last one is null, it is a
> +    // variadic function.
> +    if (FnArgs.getNumElements() > 1 &&
> +        !resolve(FnArgs.getElement(FnArgs.getNumElements() - 1)))

I don't think there's a need for the resolve call here, since it's
just a null check.

>        Children.push_back(
>            make_unique<DIE>(dwarf::DW_TAG_unspecified_parameters));
> -    }
>    }
>
>    // Collect lexical scope children first.
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=214189&r1=214188&r2=214189&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Tue Jul 29 13:20:39 2014
> @@ -1132,7 +1132,7 @@ void DwarfUnit::constructTypeDIE(DIE &Bu
>  void DwarfUnit::constructSubprogramArguments(DIE &Buffer, DITypeArray Args) {
>    for (unsigned i = 1, N = Args.getNumElements(); i < N; ++i) {
>      DIType Ty = resolve(Args.getElement(i));
> -    if (Ty.isUnspecifiedParameter()) {
> +    if (!Ty) {
>        assert(i == N-1 && "Unspecified parameter must be the last argument");
>        createAndAddDIE(dwarf::DW_TAG_unspecified_parameters, Buffer);
>      } else {
> @@ -1168,7 +1168,7 @@ void DwarfUnit::constructTypeDIE(DIE &Bu
>
>      bool isPrototyped = true;
>      if (Elements.getNumElements() == 2 &&
> -        resolve(Elements.getElement(1)).isUnspecifiedParameter())
> +        !resolve(Elements.getElement(1)))

And here (no need for the resolve call).

>        isPrototyped = false;
>
>      constructSubprogramArguments(Buffer, Elements);
>
> Modified: llvm/trunk/lib/IR/DIBuilder.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=214189&r1=214188&r2=214189&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/DIBuilder.cpp (original)
> +++ llvm/trunk/lib/IR/DIBuilder.cpp Tue Jul 29 13:20:39 2014
> @@ -875,11 +875,8 @@ void DIBuilder::retainType(DIType T) {
>
>  /// createUnspecifiedParameter - Create unspeicified type descriptor
>  /// for the subroutine type.
> -DITrivialType DIBuilder::createUnspecifiedParameter() {
> -  Value *Elts[] = {
> -    GetTagConstant(VMContext, dwarf::DW_TAG_unspecified_parameters)
> -  };
> -  return DITrivialType(MDNode::get(VMContext, Elts));
> +DIBasicType DIBuilder::createUnspecifiedParameter() {
> +  return DIBasicType(nullptr);

nullptr is the default argument, so you can omit it (just "return
DIBasicType()").

>  }
>
>  /// 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=214189&r1=214188&r2=214189&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/DebugInfo.cpp (original)
> +++ llvm/trunk/lib/IR/DebugInfo.cpp Tue Jul 29 13:20:39 2014
> @@ -39,7 +39,6 @@ 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() ||
> @@ -155,10 +154,6 @@ MDNode *DIVariable::getInlinedAt() const
>  // Predicates
>  //===----------------------------------------------------------------------===//
>
> -bool DIDescriptor::isTrivialType() const {
> -  return DbgNode && getTag() == dwarf::DW_TAG_unspecified_parameters;
> -}
> -
>  bool DIDescriptor::isSubroutineType() const {
>    return isCompositeType() && getTag() == dwarf::DW_TAG_subroutine_type;
>  }
> @@ -233,8 +228,7 @@ bool DIDescriptor::isVariable() const {
>
>  /// isType - Return true if the specified tag is legal for DIType.
>  bool DIDescriptor::isType() const {
> -  return isBasicType() || isCompositeType() || isDerivedType() ||
> -         isTrivialType();
> +  return isBasicType() || isCompositeType() || isDerivedType();
>  }
>
>  /// isSubprogram - Return true if the specified tag is legal for
> @@ -250,12 +244,6 @@ bool DIDescriptor::isGlobalVariable() co
>                       getTag() == dwarf::DW_TAG_constant);
>  }
>
> -/// isUnspecifiedParmeter - Return true if the specified tag is
> -/// DW_TAG_unspecified_parameters.
> -bool DIDescriptor::isUnspecifiedParameter() const {
> -  return DbgNode && getTag() == dwarf::DW_TAG_unspecified_parameters;
> -}
> -
>  /// isScope - Return true if the specified tag is one of the scope
>  /// related tag.
>  bool DIDescriptor::isScope() const {
> @@ -459,7 +447,7 @@ bool DIType::Verify() const {
>
>    // FIXME: Sink this into the various subclass verifies.
>    uint16_t Tag = getTag();
> -  if (!isBasicType() && !isTrivialType() && Tag != dwarf::DW_TAG_const_type &&
> +  if (!isBasicType() && 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 &&
> @@ -474,8 +462,6 @@ 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())
> @@ -489,10 +475,6 @@ 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.
> @@ -1297,7 +1279,7 @@ void DIEnumerator::printInternal(raw_ost
>  }
>
>  void DIType::printInternal(raw_ostream &OS) const {
> -  if (!DbgNode || isTrivialType())
> +  if (!DbgNode)
>      return;
>
>    StringRef Res = getName();
>
> Modified: llvm/trunk/test/DebugInfo/varargs.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/varargs.ll?rev=214189&r1=214188&r2=214189&view=diff
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/varargs.ll (original)
> +++ llvm/trunk/test/DebugInfo/varargs.ll Tue Jul 29 13:20:39 2014
> @@ -76,16 +76,15 @@ attributes #1 = { nounwind readnone }
>  !5 = metadata !{metadata !6}
>  !6 = metadata !{i32 786478, metadata !1, metadata !"_ZTS1A", metadata !"a", metadata !"a", metadata !"_ZN1A1aEiz", i32 6, metadata !7, i1 false, i1 false, i32 0, i32 0, null, i32 256, i1 false, null, null, i32 0, metadata !12, i32 6} ; [ DW_TAG_subprogram ] [line 6] [a]
>  !7 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !8, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
> -!8 = metadata !{null, metadata !9, metadata !10, metadata !11}
> +!8 = metadata !{null, metadata !9, metadata !10, null}
>  !9 = metadata !{i32 786447, null, null, metadata !"", i32 0, i64 64, i64 64, i64 0, i32 1088, metadata !"_ZTS1A"} ; [ DW_TAG_pointer_type ] [line 0, size 64, align 64, offset 0] [artificial] [from _ZTS1A]
>  !10 = metadata !{i32 786468, null, null, metadata !"int", i32 0, i64 32, i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [int] [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]
> -!11 = metadata !{i32 786456}
>  !12 = metadata !{i32 786468}
>  !13 = metadata !{metadata !14}
>  !14 = metadata !{i32 786478, metadata !1, metadata !15, metadata !"b", metadata !"b", metadata !"_Z1biz", i32 13, metadata !16, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 false, void (i32, ...)* @_Z1biz, null, null, metadata !2, i32 13} ; [ DW_TAG_subprogram ] [line 13] [def] [b]
>  !15 = metadata !{i32 786473, metadata !1}         ; [ DW_TAG_file_type ] [llvm/tools/clang/test/CodeGenCXX/debug-info-varargs.cpp]
>  !16 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !17, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
> -!17 = metadata !{null, metadata !10, metadata !11}
> +!17 = metadata !{null, metadata !10, null}
>  !18 = metadata !{i32 2, metadata !"Dwarf Version", i32 2}
>  !19 = metadata !{i32 1, metadata !"Debug Info Version", i32 1}
>  !20 = metadata !{metadata !"clang version 3.5 "}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list