[llvm] r192378 - Debug Info: In DIBuilder, the context field of subprogram is updated to use
Manman Ren
manman.ren at gmail.com
Fri Oct 11 12:55:45 PDT 2013
In my effort of writing a testing case for BreakpointPrinter, I noticed
that the printer goes through llvm.dbg.sp.
I believe that we should not directly use "llvm.dbg.sp". Is this class
still useful?
Manman
On Fri, Oct 11, 2013 at 12:12 PM, Manman Ren <manman.ren at gmail.com> wrote:
>
>
>
> On Fri, Oct 11, 2013 at 12:10 PM, David Blaikie <dblaikie at gmail.com>wrote:
>
>>
>>
>>
>> On Fri, Oct 11, 2013 at 12:04 PM, Manman Ren <manman.ren at gmail.com>wrote:
>>
>>>
>>>
>>>
>>> On Fri, Oct 11, 2013 at 11:55 AM, David Blaikie <dblaikie at gmail.com>wrote:
>>>
>>>>
>>>>
>>>>
>>>> On Fri, Oct 11, 2013 at 11:50 AM, Manman Ren <manman.ren at gmail.com>wrote:
>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Thu, Oct 10, 2013 at 11:49 AM, David Blaikie <dblaikie at gmail.com>wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Oct 10, 2013 at 11:40 AM, Manman Ren <manman.ren at gmail.com>wrote:
>>>>>>
>>>>>>> Author: mren
>>>>>>> Date: Thu Oct 10 13:40:01 2013
>>>>>>> New Revision: 192378
>>>>>>>
>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=192378&view=rev
>>>>>>> Log:
>>>>>>> Debug Info: In DIBuilder, the context field of subprogram is updated
>>>>>>> to use
>>>>>>> DIScopeRef.
>>>>>>>
>>>>>>> A paired commit at clang is required due to changes to DIBuilder.
>>>>>>>
>>>>>>> Modified:
>>>>>>> llvm/trunk/include/llvm/DIBuilder.h
>>>>>>> llvm/trunk/include/llvm/DebugInfo.h
>>>>>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>>>>>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>>>>>> llvm/trunk/lib/IR/DIBuilder.cpp
>>>>>>> llvm/trunk/lib/IR/DebugInfo.cpp
>>>>>>> llvm/trunk/test/DebugInfo/tu-composite.ll
>>>>>>> llvm/trunk/tools/opt/opt.cpp
>>>>>>>
>>>>>>> Modified: llvm/trunk/include/llvm/DIBuilder.h
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DIBuilder.h?rev=192378&r1=192377&r2=192378&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/include/llvm/DIBuilder.h (original)
>>>>>>> +++ llvm/trunk/include/llvm/DIBuilder.h Thu Oct 10 13:40:01 2013
>>>>>>> @@ -562,6 +562,20 @@ namespace llvm {
>>>>>>> MDNode *TParam = 0,
>>>>>>> MDNode *Decl = 0);
>>>>>>>
>>>>>>> + /// FIXME: this is added for dragonegg. Once we update dragonegg
>>>>>>> + /// to call resolve function, this will be removed.
>>>>>>> + DISubprogram createFunction(DIScopeRef Scope, StringRef Name,
>>>>>>> + StringRef LinkageName,
>>>>>>> + DIFile File, unsigned LineNo,
>>>>>>> + DICompositeType Ty, bool
>>>>>>> isLocalToUnit,
>>>>>>> + bool isDefinition,
>>>>>>> + unsigned ScopeLine,
>>>>>>> + unsigned Flags = 0,
>>>>>>> + bool isOptimized = false,
>>>>>>> + Function *Fn = 0,
>>>>>>> + MDNode *TParam = 0,
>>>>>>> + MDNode *Decl = 0);
>>>>>>> +
>>>>>>> /// createMethod - Create a new descriptor for the specified
>>>>>>> C++ method.
>>>>>>> /// See comments in DISubprogram for descriptions of these
>>>>>>> fields.
>>>>>>> /// @param Scope Function scope.
>>>>>>>
>>>>>>> Modified: llvm/trunk/include/llvm/DebugInfo.h
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo.h?rev=192378&r1=192377&r2=192378&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/include/llvm/DebugInfo.h (original)
>>>>>>> +++ llvm/trunk/include/llvm/DebugInfo.h Thu Oct 10 13:40:01 2013
>>>>>>> @@ -434,7 +434,7 @@ class DISubprogram : public DIScope {
>>>>>>> public:
>>>>>>> explicit DISubprogram(const MDNode *N = 0) : DIScope(N) {}
>>>>>>>
>>>>>>> - DIScope getContext() const { return getFieldAs<DIScope>(2); }
>>>>>>> + DIScopeRef getContext() const { return getFieldAs<DIScopeRef>(2);
>>>>>>> }
>>>>>>> StringRef getName() const { return getStringField(3); }
>>>>>>> StringRef getDisplayName() const { return getStringField(4); }
>>>>>>> StringRef getLinkageName() const { return getStringField(5); }
>>>>>>>
>>>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=192378&r1=192377&r2=192378&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (original)
>>>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp Thu Oct
>>>>>>> 10 13:40:01 2013
>>>>>>> @@ -1300,7 +1300,7 @@ DIE *CompileUnit::getOrCreateSubprogramD
>>>>>>> // Construct the context before querying for the existence of the
>>>>>>> DIE in case
>>>>>>> // such construction creates the DIE (as is the case for member
>>>>>>> function
>>>>>>> // declarations).
>>>>>>> - DIE *ContextDIE = getOrCreateContextDIE(SP.getContext());
>>>>>>> + DIE *ContextDIE = getOrCreateContextDIE(resolve(SP.getContext()));
>>>>>>> if (!ContextDIE)
>>>>>>> ContextDIE = CUDie.get();
>>>>>>>
>>>>>>>
>>>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=192378&r1=192377&r2=192378&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>>>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Thu Oct 10
>>>>>>> 13:40:01 2013
>>>>>>> @@ -393,9 +393,10 @@ DIE *DwarfDebug::updateSubprogramScopeDI
>>>>>>> // function then gdb prefers the definition at top level and
>>>>>>> but does not
>>>>>>> // expect specification DIE in parent function. So avoid
>>>>>>> creating
>>>>>>> // specification DIE for a function defined inside a function.
>>>>>>> - if (SP.isDefinition() && !SP.getContext().isCompileUnit() &&
>>>>>>> - !SP.getContext().isFile() &&
>>>>>>> - !isSubprogramContext(SP.getContext())) {
>>>>>>> + DIScope SPContext = resolve(SP.getContext());
>>>>>>> + if (SP.isDefinition() && !SPContext.isCompileUnit() &&
>>>>>>> + !SPContext.isFile() &&
>>>>>>> + !isSubprogramContext(SPContext)) {
>>>>>>> SPCU->addFlag(SPDie, dwarf::DW_AT_declaration);
>>>>>>>
>>>>>>> // Add arguments.
>>>>>>>
>>>>>>> Modified: llvm/trunk/lib/IR/DIBuilder.cpp
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=192378&r1=192377&r2=192378&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/lib/IR/DIBuilder.cpp (original)
>>>>>>> +++ llvm/trunk/lib/IR/DIBuilder.cpp Thu Oct 10 13:40:01 2013
>>>>>>> @@ -1041,6 +1041,28 @@ DIVariable DIBuilder::createComplexVaria
>>>>>>> }
>>>>>>>
>>>>>>> /// createFunction - Create a new descriptor for the specified
>>>>>>> function.
>>>>>>> +/// FIXME: this is added for dragonegg. Once we update dragonegg
>>>>>>> +/// to call resolve function, this will be removed.
>>>>>>> +DISubprogram DIBuilder::createFunction(DIScopeRef Context,
>>>>>>> + StringRef Name,
>>>>>>> + StringRef LinkageName,
>>>>>>> + DIFile File, unsigned LineNo,
>>>>>>> + DICompositeType Ty,
>>>>>>> + bool isLocalToUnit, bool
>>>>>>> isDefinition,
>>>>>>> + unsigned ScopeLine,
>>>>>>> + unsigned Flags, bool
>>>>>>> isOptimized,
>>>>>>> + Function *Fn,
>>>>>>> + MDNode *TParams,
>>>>>>> + MDNode *Decl) {
>>>>>>> + // dragonegg does not generate identifier for types, so using an
>>>>>>> empty map
>>>>>>> + // to resolve the context should be fine.
>>>>>>> + DITypeIdentifierMap EmptyMap;
>>>>>>> + return createFunction(Context.resolve(EmptyMap), Name,
>>>>>>> LinkageName, File,
>>>>>>> + LineNo, Ty, isLocalToUnit, isDefinition,
>>>>>>> ScopeLine,
>>>>>>> + Flags, isOptimized, Fn, TParams, Decl);
>>>>>>> +}
>>>>>>> +
>>>>>>> +/// createFunction - Create a new descriptor for the specified
>>>>>>> function.
>>>>>>> DISubprogram DIBuilder::createFunction(DIDescriptor Context,
>>>>>>> StringRef Name,
>>>>>>> StringRef LinkageName,
>>>>>>> @@ -1058,7 +1080,7 @@ DISubprogram DIBuilder::createFunction(D
>>>>>>> Value *Elts[] = {
>>>>>>> GetTagConstant(VMContext, dwarf::DW_TAG_subprogram),
>>>>>>> File.getFileNode(),
>>>>>>> - getNonCompileUnitScope(Context),
>>>>>>> + DIScope(getNonCompileUnitScope(Context)).getRef(),
>>>>>>> MDString::get(VMContext, Name),
>>>>>>> MDString::get(VMContext, Name),
>>>>>>> MDString::get(VMContext, LinkageName),
>>>>>>> @@ -1107,7 +1129,7 @@ DISubprogram DIBuilder::createMethod(DID
>>>>>>> Value *Elts[] = {
>>>>>>> GetTagConstant(VMContext, dwarf::DW_TAG_subprogram),
>>>>>>> F.getFileNode(),
>>>>>>> - getNonCompileUnitScope(Context),
>>>>>>> + DIScope(getNonCompileUnitScope(Context)).getRef(),
>>>>>>> MDString::get(VMContext, Name),
>>>>>>> MDString::get(VMContext, Name),
>>>>>>> MDString::get(VMContext, LinkageName),
>>>>>>>
>>>>>>> Modified: llvm/trunk/lib/IR/DebugInfo.cpp
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=192378&r1=192377&r2=192378&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/lib/IR/DebugInfo.cpp (original)
>>>>>>> +++ llvm/trunk/lib/IR/DebugInfo.cpp Thu Oct 10 13:40:01 2013
>>>>>>> @@ -511,8 +511,8 @@ bool DISubprogram::Verify() const {
>>>>>>> if (!isSubprogram())
>>>>>>> return false;
>>>>>>>
>>>>>>> - // Make sure context @ field 2 and type @ field 7 are MDNodes.
>>>>>>> - if (!fieldIsMDNode(DbgNode, 2))
>>>>>>> + // Make sure context @ field 2 is a ScopeRef and type @ field 7
>>>>>>> is a MDNode.
>>>>>>> + if (!fieldIsScopeRef(DbgNode, 2))
>>>>>>> return false;
>>>>>>> if (!fieldIsMDNode(DbgNode, 7))
>>>>>>> return false;
>>>>>>> @@ -1071,7 +1071,7 @@ void DebugInfoFinder::processLexicalBloc
>>>>>>> void DebugInfoFinder::processSubprogram(DISubprogram SP) {
>>>>>>> if (!addSubprogram(SP))
>>>>>>> return;
>>>>>>> - processScope(SP.getContext());
>>>>>>> + processScope(SP.getContext().resolve(TypeIdentifierMap));
>>>>>>> processType(SP.getType());
>>>>>>> DIArray TParams = SP.getTemplateParams();
>>>>>>> for (unsigned I = 0, E = TParams.getNumElements(); I != E; ++I) {
>>>>>>>
>>>>>>> Modified: llvm/trunk/test/DebugInfo/tu-composite.ll
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/tu-composite.ll?rev=192378&r1=192377&r2=192378&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/test/DebugInfo/tu-composite.ll (original)
>>>>>>> +++ llvm/trunk/test/DebugInfo/tu-composite.ll Thu Oct 10 13:40:01
>>>>>>> 2013
>>>>>>> @@ -6,9 +6,16 @@
>>>>>>> ; Make sure we correctly handle containing type of a struct being a
>>>>>>> type identifier.
>>>>>>> ; CHECK-NEXT: DW_AT_containing_type [DW_FORM_ref4] (cu +
>>>>>>> {{.*}} => {[[TYPE]]})
>>>>>>> ; CHECK-NEXT: DW_AT_name [DW_FORM_strp] {{.*}}= "C")
>>>>>>> +
>>>>>>> +; Make sure we correctly handle context of a subprogram being a
>>>>>>> type identifier.
>>>>>>> ; CHECK: [[SP:.*]]: DW_TAG_subprogram
>>>>>>> +; CHECK: DW_AT_name [DW_FORM_strp] {{.*}}= "foo")
>>>>>>> ; Make sure we correctly handle containing type of a subprogram
>>>>>>> being a type identifier.
>>>>>>> ; CHECK: DW_AT_containing_type [DW_FORM_ref4] (cu + {{.*}} =>
>>>>>>> {[[TYPE]]})
>>>>>>> +; CHECK: DW_TAG_formal_parameter
>>>>>>> +; CHECK: NULL
>>>>>>> +; CHECK: NULL
>>>>>>> +
>>>>>>> ; CHECK: [[TYPE2:.*]]: DW_TAG_structure_type
>>>>>>> ; CHECK: DW_TAG_structure_type
>>>>>>> ; CHECK: DW_AT_name [DW_FORM_strp] {{.*}}= "D")
>>>>>>> @@ -128,7 +135,7 @@ attributes #1 = { nounwind readnone }
>>>>>>> !10 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64
>>>>>>> 0, i64 0, i64 0, i32 0, null, metadata !11, i32 0, null, null, null} ; [
>>>>>>> DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
>>>>>>> !11 = metadata !{metadata !12}
>>>>>>> !12 = 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]
>>>>>>> -!13 = metadata !{i32 786478, metadata !1, metadata !4, metadata
>>>>>>> !"foo", metadata !"foo", metadata !"_ZN1C3fooEv", i32 2, metadata !14, i1
>>>>>>> false, i1 false, i32 1, i32 0, metadata !"_ZTS1C", i32 256, i1 false, null,
>>>>>>> null, i32 0, metadata !17, i32 2} ; [ DW_TAG_subprogram ] [line 2] [foo]
>>>>>>> +!13 = metadata !{i32 786478, metadata !1, metadata !"_ZTS1C",
>>>>>>> metadata !"foo", metadata !"foo", metadata !"_ZN1C3fooEv", i32 2, metadata
>>>>>>> !14, i1 false, i1 false, i32 1, i32 0, metadata !"_ZTS1C", i32 256, i1
>>>>>>> false, null, null, i32 0, metadata !17, i32 2} ; [ DW_TAG_subprogram ]
>>>>>>> [line 2] [foo]
>>>>>>> !14 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64
>>>>>>> 0, i64 0, i64 0, i32 0, null, metadata !15, i32 0, null, null, null} ; [
>>>>>>> DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
>>>>>>> !15 = metadata !{null, metadata !16}
>>>>>>> !16 = metadata !{i32 786447, null, null, metadata !"", i32 0, i64
>>>>>>> 64, i64 64, i64 0, i32 1088, metadata !"_ZTS1C"} ; [ DW_TAG_pointer_type ]
>>>>>>> [line 0, size 64, align 64, offset 0] [artificial] [from _ZTS1C]
>>>>>>>
>>>>>>> Modified: llvm/trunk/tools/opt/opt.cpp
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/opt.cpp?rev=192378&r1=192377&r2=192378&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/tools/opt/opt.cpp (original)
>>>>>>> +++ llvm/trunk/tools/opt/opt.cpp Thu Oct 10 13:40:01 2013
>>>>>>> @@ -404,7 +404,7 @@ struct BreakpointPrinter : public Module
>>>>>>> "A MDNode in llvm.dbg.sp should be null or a
>>>>>>> DISubprogram.");
>>>>>>> if (!SP)
>>>>>>> continue;
>>>>>>> - getContextName(SP.getContext(), Name);
>>>>>>> + getContextName(SP.getContext().resolve(TypeIdentifierMap),
>>>>>>> Name);
>>>>>>>
>>>>>>
>>>>>> Is there test coverage for this?
>>>>>>
>>>>>
>>>>> As long as there existed testing cases for the BreakpointPrinter, this
>>>>> change should be tested.
>>>>> I assumed we had test coverage for the BreakpointPrinter, but it seems
>>>>> that we don't by "grep -r print-breakpoints-for-testing".
>>>>>
>>>>
>>>> Put simply, if I remove this line of code, does anything fail? If not,
>>>> it'd be nice if you could add some coverage. Not strictly necessary, the
>>>> API change is simple and mechanical, etc.
>>>>
>>> Yes, the compilation will fail :)
>>>
>>
>> I'm not sure I follow - you mean opt.cpp will fail to compile if the
>> getContextName call is not there? Why is that?
>>
> I thought you meant removing the line of change. By doing grep in the test
> directory, I don't see any test that is testing it.
> But your way works too in checking whether the code is tested.
>
> Manman
>
>
>>
>>
>>> I will add a testing case to test the BreakpointPrinter.
>>>
>>
>> Thanks!
>>
>> - David
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131011/1a7970fd/attachment.html>
More information about the llvm-commits
mailing list