[llvm] r192378 - Debug Info: In DIBuilder, the context field of subprogram is updated to use
David Blaikie
dblaikie at gmail.com
Fri Oct 11 12:59:11 PDT 2013
On Fri, Oct 11, 2013 at 12:55 PM, Manman Ren <manman.ren at gmail.com> wrote:
> 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?
>
I don't really know what this class is/where its functionality is exposed,
but if it seems broken then perhaps you could just send a CR to remove the
functionality on the grounds that it's bitrotted for quite a while now. If
no one objects, we'll just remove it and leave it to someone else to fix
correctly if they ever decide they need it.
- David
>
> 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/3fc94971/attachment.html>
More information about the llvm-commits
mailing list