<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Oct 11, 2013 at 12:04 PM, Manman Ren <span dir="ltr"><<a href="mailto:manman.ren@gmail.com" target="_blank">manman.ren@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">On Fri, Oct 11, 2013 at 11:55 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div>On Fri, Oct 11, 2013 at 11:50 AM, Manman Ren <span dir="ltr"><<a href="mailto:manman.ren@gmail.com" target="_blank">manman.ren@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div>On Thu, Oct 10, 2013 at 11:49 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">
<div><div>On Thu, Oct 10, 2013 at 11:40 AM, Manman Ren <span dir="ltr"><<a href="mailto:manman.ren@gmail.com" target="_blank">manman.ren@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: mren<br>
Date: Thu Oct 10 13:40:01 2013<br>
New Revision: 192378<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=192378&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=192378&view=rev</a><br>
Log:<br>
Debug Info: In DIBuilder, the context field of subprogram is updated to use<br>
DIScopeRef.<br>
<br>
A paired commit at clang is required due to changes to DIBuilder.<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/DIBuilder.h<br>
llvm/trunk/include/llvm/DebugInfo.h<br>
llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp<br>
llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
llvm/trunk/lib/IR/DIBuilder.cpp<br>
llvm/trunk/lib/IR/DebugInfo.cpp<br>
llvm/trunk/test/DebugInfo/tu-composite.ll<br>
llvm/trunk/tools/opt/opt.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/DIBuilder.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DIBuilder.h?rev=192378&r1=192377&r2=192378&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DIBuilder.h?rev=192378&r1=192377&r2=192378&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/DIBuilder.h (original)<br>
+++ llvm/trunk/include/llvm/DIBuilder.h Thu Oct 10 13:40:01 2013<br>
@@ -562,6 +562,20 @@ namespace llvm {<br>
MDNode *TParam = 0,<br>
MDNode *Decl = 0);<br>
<br>
+ /// FIXME: this is added for dragonegg. Once we update dragonegg<br>
+ /// to call resolve function, this will be removed.<br>
+ DISubprogram createFunction(DIScopeRef Scope, StringRef Name,<br>
+ StringRef LinkageName,<br>
+ DIFile File, unsigned LineNo,<br>
+ DICompositeType Ty, bool isLocalToUnit,<br>
+ bool isDefinition,<br>
+ unsigned ScopeLine,<br>
+ unsigned Flags = 0,<br>
+ bool isOptimized = false,<br>
+ Function *Fn = 0,<br>
+ MDNode *TParam = 0,<br>
+ MDNode *Decl = 0);<br>
+<br>
/// createMethod - Create a new descriptor for the specified C++ method.<br>
/// See comments in DISubprogram for descriptions of these fields.<br>
/// @param Scope Function scope.<br>
<br>
Modified: llvm/trunk/include/llvm/DebugInfo.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo.h?rev=192378&r1=192377&r2=192378&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo.h?rev=192378&r1=192377&r2=192378&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/DebugInfo.h (original)<br>
+++ llvm/trunk/include/llvm/DebugInfo.h Thu Oct 10 13:40:01 2013<br>
@@ -434,7 +434,7 @@ class DISubprogram : public DIScope {<br>
public:<br>
explicit DISubprogram(const MDNode *N = 0) : DIScope(N) {}<br>
<br>
- DIScope getContext() const { return getFieldAs<DIScope>(2); }<br>
+ DIScopeRef getContext() const { return getFieldAs<DIScopeRef>(2); }<br>
StringRef getName() const { return getStringField(3); }<br>
StringRef getDisplayName() const { return getStringField(4); }<br>
StringRef getLinkageName() const { return getStringField(5); }<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=192378&r1=192377&r2=192378&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=192378&r1=192377&r2=192378&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp Thu Oct 10 13:40:01 2013<br>
@@ -1300,7 +1300,7 @@ DIE *CompileUnit::getOrCreateSubprogramD<br>
// Construct the context before querying for the existence of the DIE in case<br>
// such construction creates the DIE (as is the case for member function<br>
// declarations).<br>
- DIE *ContextDIE = getOrCreateContextDIE(SP.getContext());<br>
+ DIE *ContextDIE = getOrCreateContextDIE(resolve(SP.getContext()));<br>
if (!ContextDIE)<br>
ContextDIE = CUDie.get();<br>
<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=192378&r1=192377&r2=192378&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=192378&r1=192377&r2=192378&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Thu Oct 10 13:40:01 2013<br>
@@ -393,9 +393,10 @@ DIE *DwarfDebug::updateSubprogramScopeDI<br>
// function then gdb prefers the definition at top level and but does not<br>
// expect specification DIE in parent function. So avoid creating<br>
// specification DIE for a function defined inside a function.<br>
- if (SP.isDefinition() && !SP.getContext().isCompileUnit() &&<br>
- !SP.getContext().isFile() &&<br>
- !isSubprogramContext(SP.getContext())) {<br>
+ DIScope SPContext = resolve(SP.getContext());<br>
+ if (SP.isDefinition() && !SPContext.isCompileUnit() &&<br>
+ !SPContext.isFile() &&<br>
+ !isSubprogramContext(SPContext)) {<br>
SPCU->addFlag(SPDie, dwarf::DW_AT_declaration);<br>
<br>
// Add arguments.<br>
<br>
Modified: llvm/trunk/lib/IR/DIBuilder.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=192378&r1=192377&r2=192378&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=192378&r1=192377&r2=192378&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/DIBuilder.cpp (original)<br>
+++ llvm/trunk/lib/IR/DIBuilder.cpp Thu Oct 10 13:40:01 2013<br>
@@ -1041,6 +1041,28 @@ DIVariable DIBuilder::createComplexVaria<br>
}<br>
<br>
/// createFunction - Create a new descriptor for the specified function.<br>
+/// FIXME: this is added for dragonegg. Once we update dragonegg<br>
+/// to call resolve function, this will be removed.<br>
+DISubprogram DIBuilder::createFunction(DIScopeRef Context,<br>
+ StringRef Name,<br>
+ StringRef LinkageName,<br>
+ DIFile File, unsigned LineNo,<br>
+ DICompositeType Ty,<br>
+ bool isLocalToUnit, bool isDefinition,<br>
+ unsigned ScopeLine,<br>
+ unsigned Flags, bool isOptimized,<br>
+ Function *Fn,<br>
+ MDNode *TParams,<br>
+ MDNode *Decl) {<br>
+ // dragonegg does not generate identifier for types, so using an empty map<br>
+ // to resolve the context should be fine.<br>
+ DITypeIdentifierMap EmptyMap;<br>
+ return createFunction(Context.resolve(EmptyMap), Name, LinkageName, File,<br>
+ LineNo, Ty, isLocalToUnit, isDefinition, ScopeLine,<br>
+ Flags, isOptimized, Fn, TParams, Decl);<br>
+}<br>
+<br>
+/// createFunction - Create a new descriptor for the specified function.<br>
DISubprogram DIBuilder::createFunction(DIDescriptor Context,<br>
StringRef Name,<br>
StringRef LinkageName,<br>
@@ -1058,7 +1080,7 @@ DISubprogram DIBuilder::createFunction(D<br>
Value *Elts[] = {<br>
GetTagConstant(VMContext, dwarf::DW_TAG_subprogram),<br>
File.getFileNode(),<br>
- getNonCompileUnitScope(Context),<br>
+ DIScope(getNonCompileUnitScope(Context)).getRef(),<br>
MDString::get(VMContext, Name),<br>
MDString::get(VMContext, Name),<br>
MDString::get(VMContext, LinkageName),<br>
@@ -1107,7 +1129,7 @@ DISubprogram DIBuilder::createMethod(DID<br>
Value *Elts[] = {<br>
GetTagConstant(VMContext, dwarf::DW_TAG_subprogram),<br>
F.getFileNode(),<br>
- getNonCompileUnitScope(Context),<br>
+ DIScope(getNonCompileUnitScope(Context)).getRef(),<br>
MDString::get(VMContext, Name),<br>
MDString::get(VMContext, Name),<br>
MDString::get(VMContext, LinkageName),<br>
<br>
Modified: llvm/trunk/lib/IR/DebugInfo.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=192378&r1=192377&r2=192378&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=192378&r1=192377&r2=192378&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/DebugInfo.cpp (original)<br>
+++ llvm/trunk/lib/IR/DebugInfo.cpp Thu Oct 10 13:40:01 2013<br>
@@ -511,8 +511,8 @@ bool DISubprogram::Verify() const {<br>
if (!isSubprogram())<br>
return false;<br>
<br>
- // Make sure context @ field 2 and type @ field 7 are MDNodes.<br>
- if (!fieldIsMDNode(DbgNode, 2))<br>
+ // Make sure context @ field 2 is a ScopeRef and type @ field 7 is a MDNode.<br>
+ if (!fieldIsScopeRef(DbgNode, 2))<br>
return false;<br>
if (!fieldIsMDNode(DbgNode, 7))<br>
return false;<br>
@@ -1071,7 +1071,7 @@ void DebugInfoFinder::processLexicalBloc<br>
void DebugInfoFinder::processSubprogram(DISubprogram SP) {<br>
if (!addSubprogram(SP))<br>
return;<br>
- processScope(SP.getContext());<br>
+ processScope(SP.getContext().resolve(TypeIdentifierMap));<br>
processType(SP.getType());<br>
DIArray TParams = SP.getTemplateParams();<br>
for (unsigned I = 0, E = TParams.getNumElements(); I != E; ++I) {<br>
<br>
Modified: llvm/trunk/test/DebugInfo/tu-composite.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/tu-composite.ll?rev=192378&r1=192377&r2=192378&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/tu-composite.ll?rev=192378&r1=192377&r2=192378&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/DebugInfo/tu-composite.ll (original)<br>
+++ llvm/trunk/test/DebugInfo/tu-composite.ll Thu Oct 10 13:40:01 2013<br>
@@ -6,9 +6,16 @@<br>
; Make sure we correctly handle containing type of a struct being a type identifier.<br>
; CHECK-NEXT: DW_AT_containing_type [DW_FORM_ref4] (cu + {{.*}} => {[[TYPE]]})<br>
; CHECK-NEXT: DW_AT_name [DW_FORM_strp] {{.*}}= "C")<br>
+<br>
+; Make sure we correctly handle context of a subprogram being a type identifier.<br>
; CHECK: [[SP:.*]]: DW_TAG_subprogram<br>
+; CHECK: DW_AT_name [DW_FORM_strp] {{.*}}= "foo")<br>
; Make sure we correctly handle containing type of a subprogram being a type identifier.<br>
; CHECK: DW_AT_containing_type [DW_FORM_ref4] (cu + {{.*}} => {[[TYPE]]})<br>
+; CHECK: DW_TAG_formal_parameter<br>
+; CHECK: NULL<br>
+; CHECK: NULL<br>
+<br>
; CHECK: [[TYPE2:.*]]: DW_TAG_structure_type<br>
; CHECK: DW_TAG_structure_type<br>
; CHECK: DW_AT_name [DW_FORM_strp] {{.*}}= "D")<br>
@@ -128,7 +135,7 @@ attributes #1 = { nounwind readnone }<br>
!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 ]<br>
!11 = metadata !{metadata !12}<br>
!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]<br>
-!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]<br>
+!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]<br>
!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 ]<br>
!15 = metadata !{null, metadata !16}<br>
!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]<br>
<br>
Modified: llvm/trunk/tools/opt/opt.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/opt.cpp?rev=192378&r1=192377&r2=192378&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/opt/opt.cpp?rev=192378&r1=192377&r2=192378&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/tools/opt/opt.cpp (original)<br>
+++ llvm/trunk/tools/opt/opt.cpp Thu Oct 10 13:40:01 2013<br>
@@ -404,7 +404,7 @@ struct BreakpointPrinter : public Module<br>
"A MDNode in llvm.dbg.sp should be null or a DISubprogram.");<br>
if (!SP)<br>
continue;<br>
- getContextName(SP.getContext(), Name);<br>
+ getContextName(SP.getContext().resolve(TypeIdentifierMap), Name);<br></blockquote><div><br></div></div></div><div>Is there test coverage for this?</div></div></div></div></blockquote><div><br></div></div></div><div>
As long as there existed testing cases for the BreakpointPrinter, this change should be tested.</div>
<div>I assumed we had test coverage for the BreakpointPrinter, but it seems that we don't by "grep -r print-breakpoints-for-testing".</div></div></div></div></blockquote><div><br></div></div></div><div>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. <br>
</div></div></div></div></blockquote></div></div><div>Yes, the compilation will fail :)</div></div></div></div></blockquote><div><br></div><div>I'm not sure I follow - you mean opt.cpp will fail to compile if the getContextName call is not there? Why is that?</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>I will add a testing case to test the BreakpointPrinter.</div>
</div></div></div></blockquote><div><br></div><div>Thanks!<br><br></div><div>- David </div></div></div></div>