r214133 - [Debug Info] add DISubroutineType and its creation takes DITypeArray.

David Blaikie dblaikie at gmail.com
Tue Jul 29 10:27:53 PDT 2014


On Tue, Jul 29, 2014 at 10:20 AM, Manman Ren <manman.ren at gmail.com> wrote:
>
>
>
> On Tue, Jul 29, 2014 at 8:24 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> On Mon, Jul 28, 2014 at 3:24 PM, Manman Ren <manman.ren at gmail.com> wrote:
>> > Author: mren
>> > Date: Mon Jul 28 17:24:34 2014
>> > New Revision: 214133
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=214133&view=rev
>> > Log:
>> > [Debug Info] add DISubroutineType and its creation takes DITypeArray.
>> >
>> > This is the last patch to unique the type array of a subroutine type.
>> > This is the paired commit with llvm r214132.
>> >
>> > Modified:
>> >     cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> >     cfe/trunk/test/CodeGenCXX/debug-info-template-member.cpp
>> >
>> > Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=214133&r1=214132&r2=214133&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>> > +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Jul 28 17:24:34 2014
>> > @@ -784,7 +784,7 @@ llvm::DIType CGDebugInfo::CreateType(con
>> >        EltTys.push_back(DBuilder.createUnspecifiedParameter());
>> >    }
>> >
>> > -  llvm::DIArray EltTypeArray = DBuilder.getOrCreateArray(EltTys);
>> > +  llvm::DITypeArray EltTypeArray =
>> > DBuilder.getOrCreateTypeArray(EltTys);
>> >    return DBuilder.createSubroutineType(Unit, EltTypeArray);
>> >  }
>> >
>> > @@ -986,8 +986,8 @@ CGDebugInfo::getOrCreateMethodType(const
>> >  llvm::DICompositeType CGDebugInfo::getOrCreateInstanceMethodType(
>> >      QualType ThisPtr, const FunctionProtoType *Func, llvm::DIFile Unit)
>> > {
>> >    // Add "this" pointer.
>> > -  llvm::DIArray Args = llvm::DICompositeType(
>> > -      getOrCreateType(QualType(Func, 0), Unit)).getElements();
>> > +  llvm::DITypeArray Args = llvm::DISubroutineType(
>> > +      getOrCreateType(QualType(Func, 0), Unit)).getTypeArray();
>> >    assert (Args.getNumElements() && "Invalid number of arguments!");
>> >
>> >    SmallVector<llvm::Value *, 16> Elts;
>> > @@ -1024,7 +1024,7 @@ llvm::DICompositeType CGDebugInfo::getOr
>> >    for (unsigned i = 1, e = Args.getNumElements(); i != e; ++i)
>> >      Elts.push_back(Args.getElement(i));
>> >
>> > -  llvm::DIArray EltTypeArray = DBuilder.getOrCreateArray(Elts);
>> > +  llvm::DITypeArray EltTypeArray = DBuilder.getOrCreateTypeArray(Elts);
>> >
>> >    unsigned Flags = 0;
>> >    if (Func->getExtProtoInfo().RefQualifier == RQ_LValue)
>> > @@ -1374,7 +1374,7 @@ llvm::DIType CGDebugInfo::getOrCreateVTa
>> >
>> >    /* Function type */
>> >    llvm::Value *STy = getOrCreateType(Context.IntTy, Unit);
>> > -  llvm::DIArray SElements = DBuilder.getOrCreateArray(STy);
>> > +  llvm::DITypeArray SElements = DBuilder.getOrCreateTypeArray(STy);
>> >    llvm::DIType SubTy = DBuilder.createSubroutineType(Unit, SElements);
>> >    unsigned Size = Context.getTypeSize(Context.VoidPtrTy);
>> >    llvm::DIType vtbl_ptr_type = DBuilder.createPointerType(SubTy, Size,
>> > 0,
>> > @@ -2392,7 +2392,8 @@ llvm::DICompositeType CGDebugInfo::getOr
>> >      // llvm::DISubprogram::Verify() would return false, and
>> >      // subprogram DIE will miss DW_AT_decl_file and
>> >      // DW_AT_decl_line fields.
>> > -    return DBuilder.createSubroutineType(F,
>> > DBuilder.getOrCreateArray(None));
>> > +    return DBuilder.createSubroutineType(F,
>> > +
>> > DBuilder.getOrCreateTypeArray(None));
>> >
>> >    if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D))
>> >      return getOrCreateMethodType(Method, F);
>> > @@ -2420,7 +2421,7 @@ llvm::DICompositeType CGDebugInfo::getOr
>> >      for (const auto *PI : OMethod->params())
>> >        Elts.push_back(getOrCreateType(PI->getType(), F));
>> >
>> > -    llvm::DIArray EltTypeArray = DBuilder.getOrCreateArray(Elts);
>> > +    llvm::DITypeArray EltTypeArray =
>> > DBuilder.getOrCreateTypeArray(Elts);
>> >      return DBuilder.createSubroutineType(F, EltTypeArray);
>> >    }
>> >
>> > @@ -2434,7 +2435,7 @@ llvm::DICompositeType CGDebugInfo::getOr
>> >          for (unsigned i = 0, e = FPT->getNumParams(); i != e; ++i)
>> >            EltTys.push_back(getOrCreateType(FPT->getParamType(i), F));
>> >        EltTys.push_back(DBuilder.createUnspecifiedParameter());
>> > -      llvm::DIArray EltTypeArray = DBuilder.getOrCreateArray(EltTys);
>> > +      llvm::DITypeArray EltTypeArray =
>> > DBuilder.getOrCreateTypeArray(EltTys);
>> >        return DBuilder.createSubroutineType(F, EltTypeArray);
>> >      }
>> >
>> >
>> > Modified: cfe/trunk/test/CodeGenCXX/debug-info-template-member.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-template-member.cpp?rev=214133&r1=214132&r2=214133&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/test/CodeGenCXX/debug-info-template-member.cpp (original)
>> > +++ cfe/trunk/test/CodeGenCXX/debug-info-template-member.cpp Mon Jul 28
>> > 17:24:34 2014
>> > @@ -20,8 +20,8 @@ inline int add3(int x) {
>> >  // CHECK: [[FOO_MEM]] = metadata !{metadata [[FOO_FUNC:![0-9]*]]}
>> >  // CHECK: [[FOO_FUNC]] = {{.*}}, metadata
>> > !"_ZN3foo4funcEN5outerIS_E5innerE", i32 {{[0-9]*}}, metadata
>> > [[FOO_FUNC_TYPE:![0-9]*]], {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [func]
>> >  // CHECK: [[FOO_FUNC_TYPE]] = {{.*}}, metadata
>> > [[FOO_FUNC_PARAMS:![0-9]*]], i32 0, null, null, null} ; [
>> > DW_TAG_subroutine_type ]
>> > -// CHECK: [[FOO_FUNC_PARAMS]] = metadata !{null, metadata !{{[0-9]*}},
>> > metadata [[OUTER_FOO_INNER:![0-9]*]]}
>> > -// CHECK: [[OUTER_FOO_INNER]] = {{.*}}, null, metadata
>> > !"[[OUTER_FOO_INNER_ID:.*]]"} ; [ DW_TAG_structure_type ] [inner]
>>
>> FWIW I usually match the metadata value including the ! for
>> cross-references, like this:
>>
>>   metadata [[OUTER_FOO_INNER_ID:!".*"]]
>>
>> that way the back reference doesn't need to repeat the !" stuff:
>>
>>   metadata [[OUTER_FOO_INNER_ID]]
>>
>> And the initial match still has the string bits, which ensure that it
>> is a string value, not a direct metadata reference.
>>
>> Though, given that there's a CHECK line just two lines up that uses
>> the actual mangled name (at least I assume that's the same name:
>> "_ZN3foo4funcEN5outerIS_E5innerE") perhaps it'd make more sense just
>> to use the name directly, rather than a capturing reference?
>
>
>  _ZN3foo4funcEN5outerIS_E5innerE is the mangled name for the subprogram,

Ah, so it is.

> and here we are checking the mangled name for one of the parameter type,
> so they are different.

Indeed - though I'd still say it's probably fine to use the exact
name, rather than a matching group - we know, based on the input, what
we expect the output to be (though, granted, this code shouldn't
necessarily be testing the mangler).

We certainly want to check that the two strings match, which your test
change does, but by testing the exact string we also test that the two
strings are unique in the way we expect them to be (eg: your test as
is could pass if the strings were empty (maybe - not sure if FileCheck
allows empty matches), for example, or equal to some other string).

No worries though - up to you,

- Dave

> Thanks for the post-commit review,
> Manman
>
>>
>> In the frontend you have to be careful not to match the name directly
>> due to differences in mangling between itanium and Windows, etc - but
>> that's not the case with this backend test where the mangled name is
>> already hardcoded in the input.
>>
>> > +// CHECK: [[FOO_FUNC_PARAMS]] = metadata !{null, metadata !{{[0-9]*}},
>> > metadata !"[[OUTER_FOO_INNER_ID:.*]]"}
>> > +// CHECK: !{{[0-9]*}} = {{.*}}, null, metadata
>> > !"[[OUTER_FOO_INNER_ID]]"} ; [ DW_TAG_structure_type ] [inner]
>> >
>> >  // CHECK: metadata [[VIRT_MEM:![0-9]*]], i32 0, metadata
>> > !"_ZTS4virtI4elemE", metadata [[VIRT_TEMP_PARAM:![0-9]*]], metadata
>> > !"_ZTS4virtI4elemE"} ; [ DW_TAG_structure_type ] [virt<elem>] {{.*}} [def]
>> >  // CHECK: [[VIRT_TEMP_PARAM]] = metadata !{metadata [[VIRT_T:![0-9]*]]}
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>



More information about the cfe-commits mailing list