<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, Feb 24, 2013 at 2:53 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@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">Hi Dave,<div><br></div><div>Looks pretty good, just a few comments:</div>
<div><br></div><div><div>- DW_TAG_inheritance, findRegion(TYPE_CONTEXT(type)),</div><div>+ DW_TAG_inheritance, findRegion(type),</div>
</div><div><br></div><div>This seems curious?</div></div></blockquote><div><br></div><div>Yeah, seems DragonEgg was creating DW_TAG_inheritance metadata that made basically no sense. Given a simple source file:<br>
<br>struct base {<br>};<br><br>struct derived: base {</div><div>};</div><div><br></div><div>int main() {</div><div> derived d;<br>}<br><br>The (abbreviated) old IR looks like this:<br><br>...<br>!1 = metadata !{i32 655401, metadata !"inherit.cpp", metadata !"/usr/local/google/home/blaikie/dev/cte/build/", metadata !2}<br>
...<br>!9 = metadata !{i32 655388, metadata !1, metadata !"", null, i32 0, i64 0, i64 0, i64 0, i32 0, metadata !10}<br></div><div>!10 = metadata !{i32 655379, metadata !1, metadata !"base", metadata !1, i32 1, i64 8, i64 8, i64 0, i32 0, null, metadata !11, i32 0, null}<br>
<br>Which basically says "DICompositeType 'base' derives from DIFile 'inherit.cpp'" which doesn't seem particularly useful/correct.<br><br>Once ported to DIBuilder this caused assertion failures because the DIType passed to createInheritance was really a DIFile & failed the 'is valid' check in createInheritance.</div>
<div><br></div><div>With the change the debug info looks like:<br>...</div><div>!9 = metadata !{i32 786451, metadata !4, metadata !"derived", metadata !4, i32 4, i64 8, i64 8, i32 0, i32 0, null, metadata !10, i32 0, null, null} ; [ DW_TAG_structure_type ] [derived] [line 4, size 8, align 8, offset 0] [from ]<br>
...<br>!11 = metadata !{i32 786460, metadata !9, null, null, i32 0, i64 0, i64 0, i64 0, i32 0, metadata !12} ; [ DW_TAG_inheritance ] [line 0, size 0, align 0, offset 0] [from base]<br></div><div>!12 = metadata !{i32 786451, metadata !4, metadata !"base", metadata !4, i32 1, i64 8, i64 8, i32 0, i32 0, null, metadata !1, i32 0, null, null} ; [ DW_TAG_structure_type ] [base] [line 1, size 8, align 8, offset 0] [from ]<br>
</div><div><br></div><div>"derived is derived from base" - hooray</div><div> </div><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"><div><br></div><div><div>+ case dwarf::DW_TAG_typedef:</div><div>+ // FIXME: Remove this conditional, it's firing at least for member pointer</div>
<div>+ // fix getOrCreateType to see through member data pointers</div><div><br></div><div>Can you expand on this comment (also needs a period) since it's not obvious what's going on and why?</div></div></div>
</blockquote><div><br></div><div>More detail provided:<br><br><div> // FIXME: This code should use DIBuilder but it's creating something</div><div> // invalid that DIBuilder doesn't allow. DerivedFrom is invalid due to</div>
<div> // getOrCreateType failing to provide the underlying member type for a </div><div> // pointer to member. The invalid DerivedType causes this code to create</div><div> // an invalid typedef that is a typedef of no other type. (hence the null</div>
<div> // value as the last parameter in Elts, below)</div></div><div><br></div><div style>Let me know if that's unclear/needs further detail.</div><div> </div><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"><div><div><br>
</div><div><div>+ case dwarf::DW_TAG_structure_type:</div><div>+ return Builder.createStructType(Context, Name, F, LineNumber, SizeInBits, AlignInBits, Flags, DerivedFrom, Elements, 0, ContainingType);</div></div><div>
<br></div><div>80-col?</div></div></div></blockquote><div><br></div><div>Wrapped.</div><div> </div><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"><div><div><br></div><div><div>+ return Builder.createFunction(</div><div>+ SP.getContext(), SP.getName(), SP.getLinkageName(), SP.getFile(),</div><div>+ SP.getLineNumber(), SP.getType(), SP.isLocalToUnit(), true, LineNo,</div>
<div>+ SP.getFlags(), SP.isOptimized(), Fn, SP.getTemplateParams(), SP);</div><div><br></div><div>Awkward formatting?</div><div><br></div><div><div>+ DICompositeType createStructType(</div><div>+ DIDescriptor Scope, StringRef Name, DIFile File, unsigned LineNumber,</div>
<div>+ uint64_t SizeInBits, uint64_t AlignInBits, unsigned Flags,</div><div>+ DIType DerivedFrom, DIArray Elements, unsigned RunTimeLang = 0,</div><div>+ MDNode *VTableHolder = 0);</div><div><br></div>
<div>Ditto.</div><div><br></div><div><div>+DICompositeType DIBuilder::createStructType(</div><div>+ DIDescriptor Context, StringRef Name, DIFile File, unsigned LineNumber,</div><div>+ uint64_t SizeInBits, uint64_t AlignInBits, unsigned Flags,</div>
<div>+ DIType DerivedFrom, DIArray Elements, unsigned RunTimeLang,</div><div>+ MDNode *VTableHolder) {</div><div><br></div><div>Here too.</div></div></div></div></div></div></blockquote><div><br></div><div>Formatting all made consistent with the previous style (wrapped/aligned to the '(').</div>
<div> </div><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"><div><div><div><div><div><br></div>
<div>Otherwise OK.</div></div></div></div></div></div></blockquote><div><br></div><div>Thanks - committed in 176003 (Clang), 176004 (LLVM), and r176006 (DragonEgg).<br><br>Thanks all,</div><div style>- David</div><div> </div>
<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"><div><div><div><div><span><font color="#888888"><div><br>
</div><div>-eric</div></font></span></div></div></div></div></div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, Feb 24, 2013 at 10:25 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>On Sun, Feb 24, 2013 at 12:23 AM, Duncan Sands <span dir="ltr"><<a href="mailto:baldrick@free.fr" target="_blank">baldrick@free.fr</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">Hi David, here is the original commit. </blockquote><div>
<br></div></div><div>Ah, I see - you inherited these test cases from llvm-gcc. Makes sense.</div>
<div>
<div> </div><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"> Since dragonegg just uses the provided<br>
gcc (i.e. it can't modify gcc), if gcc hasn't set DECL_IGNORED_P then there is<br>
nothing that dragonegg can do about it. I've removed the test case. Please<br>
feel free to apply your debug info patches.<br></blockquote><div><br></div></div><div>Great - thanks for your time/thoughts/fixes here. I know Eric mentioned he had some thoughts on these patches so I'll wait for him to have a chance & then commit these & carry on with my debug info refactoring work. (in doing so I'll try to keep an eye on the dragonegg test cases too - though I apologize in advance if a few breaks slip through & please do let me know if that happens)<span><font color="#888888"><br>
<br>- David</font></span></div><div><div><div> </div><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">
<br>
Ciao, Duncan.<br>
<br>
r96974 | dpatel | 2010-02-23 20:36:49 +0100 (Tue, 23 Feb 2010) | 4 lines<br>
<br>
Do not rely on write_symbols to disable debug info for super class added as an invisible member of derived class. write_symbols controls debug_hooks which are used to emit debug info in various formats in gcc. llvm-gcc does not use this debug_hooks interface to emit debugging information.<br>
<br>
Test case is at llvm/test/FrontendObjC/2010-<u></u>02-23-DbgInheritance.m<br>
<br>
<br>
Index: gcc/toplev.c<br>
==============================<u></u>==============================<u></u>=======<br>
--- gcc/toplev.c (revision 96973)<br>
+++ gcc/toplev.c (revision 96974)<br>
@@ -1926,6 +1926,8 @@<br>
<br>
/* LLVM LOCAL begin */<br>
#ifdef ENABLE_LLVM<br>
+ // write_symbols set up debug_hooks. llvm-gcc does not use this hooks<br>
+ // to emit debug info.<br>
write_symbols = NO_DEBUG;<br>
#endif<br>
/* LLVM LOCAL end */<br>
Index: gcc/objc/objc-act.c<br>
==============================<u></u>==============================<u></u>=======<br>
--- gcc/objc/objc-act.c (revision 96973)<br>
+++ gcc/objc/objc-act.c (revision 96974)<br>
@@ -3407,8 +3407,10 @@<br>
DECL_ALIGN (base) = 1;<br>
DECL_FIELD_CONTEXT (base) = s;<br>
/* APPLE LOCAL begin radar 4477797 */<br>
- if (write_symbols == DWARF2_DEBUG)<br>
- DECL_IGNORED_P (base) = 1;<br>
+ /* LLVM LOCAL begin */<br>
+ /* Do not check write-symbols in llvm-gcc. */<br>
+ DECL_IGNORED_P (base) = 1;<br>
+ /* LLVM LOCAL end */<br>
/* APPLE LOCAL end radar 4477797 */<br>
#ifdef OBJCPLUS<br>
DECL_FIELD_IS_BASE (base) = 1;<br>
<br>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>