<div dir="ltr"><br><div style>I am now a little confused about the mapping issue.</div><div style>The patch replaces every getDIE of a Type, a SP, and a Static Member with DwarfDebug.getType|SP|StaticMemberDie.</div><div style>
<br></div><div style>getDIE was used before creating a DIE, and it was also used before generating a reference to the DIE via ref4 or ref_addr.</div><div style><br></div><div style>For the usage before creating a DIE, if we still use the CU mapping, we will create multiple DIEs for the same MDNode across CUs, if we replace it with the DwarfDebug mapping, we will not create multiple DIEs.</div>
<div style><br></div><div style>For the usage before generating a reference, it does not make sense to use CU mapping if we only have a single DIE for the MDNode (i.e using DwarfDebug mapping before creating a DIE); it does not make sense to use DwarfDebug mapping if we already</div>
<div style>have multiple DIEs for the MDNode.</div><div style><br></div><div style>So it sounds to me, the usage of getDIE before creating a DIE determines whether we have multiple DIEs for a given MDNode and other usages of getDIE should either use CU mapping or DwarfDebug mapping, but not both for a given MDNode.</div>
<div style><br></div><div style>Thanks,</div><div style>Manman</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Oct 1, 2013 at 5:58 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 class="im">On Tue, Oct 1, 2013 at 4:10 PM, 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"><div class="gmail_quote">
<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 class="gmail_extra"><div class="gmail_quote">
<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 class="gmail_extra"><div class="gmail_quote">
<div>The idea is that we're going to have references across-CUs? Do you have any idea if debuggers tolerate/handle this kind of referencing?</div></div></div></div></blockquote></div><div>Yes, the DIE will be referenced across-CUs. We have been using ref_addr for some cases, </div>
</div></div></div></blockquote><div><br></div></div><div>In which cases do we already do this?</div></div></div></div></blockquote></div><div>In DwarfDebug::updateSubprogramScopeDIE, when we add abstract_origin of a subprogram.</div>
<div>I agree that we are now using ref_addr broadly for LTO builds.</div><div class="im"><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 class="gmail_extra"><div class="gmail_quote"><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 class="gmail_extra"><div class="gmail_quote"><div>and lldb is fine with it.</div></div></div></div></blockquote><div><br></div></div><div>lldb isn't the only DWARF consumer.</div></div></div></div></blockquote></div>
<div>
Other debuggers should work with ref_addr.</div><div class="im"><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 class="gmail_extra"><div class="gmail_quote"><div><div><br></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 class="gmail_extra"><div class="gmail_quote"><div>I tried to move the mapping for MDNodes related to the type system, from CompileUnit to DwarfDebug, and I didn't include namespace in this commit.</div>
<div>Theoretically, for everything that can be shared across CUs, we should add it to the DwarfDebug map to remove duplication.</div></div></div></div></blockquote><div><br></div></div><div>Then it would be nice to do this as a single, generic solution over all things that can be referenced rather than piecemeal with special cases for each kind of thing, I think.</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"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<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 class="gmail_extra"><div class="gmail_quote">
<div>
<br>So a strawman alternative proposal would be: add all the MDNode/DIE mappings to both the local CompileUnit and 'global' DwarfDebug maps and have different lookup functions depending on the context - using the DwarfDebug mapping when we want to reference anything for any reason, and the CompileUnit mapping when we want to add children (eg: adding new things to a namespace, etc) or attributes.</div>
</div></div></div></blockquote></div><div>I don't quite get why we need two mappings for one MDNode.</div></div></div></div></blockquote><div><br></div></div><div>Because, say we have a namespace node - we need a CU-specific mapping so that when we need to add entities to the namespace (another type, global variable, etc) we can lookup the CU-local instance of the namespace. But when we only need to refer to the namespace (eg: for a namespace alias) we can lookup a cross-CU mapping.<br>
</div></div></div></div></blockquote></div><div>If we already have multiple DIEs for the same MDNode (that is why we keep a CU-specific mapping, right?), when referring to the namespace DIE, can we just use CU-specific mapping?</div>
<div>I am not familiar with how a namespace alias works, so don't quite know why we need a cross-CU mapping when referring to the namespace.</div><div class="im"><div><br></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 class="gmail_extra"><div class="gmail_quote"><div>
<br>This is exactly what you've already implemented - the mappings you added to DwarfDebug are the second mapping - the first mapping already exists within the CompileUnit and is still used for the cases where we want to add members (at least I hope we are, because we need to do that in some cases).</div>
</div></div></div></blockquote></div><div>Oops, I moved the mapping from CompileUnit to DwarfDebug. Are you referring to "the namespace alias" by "some cases"? </div><div class="im"><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 class="gmail_extra"><div class="gmail_quote"><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 class="gmail_extra"><div class="gmail_quote">
<div>I tried to categorize MDNodes to two groups, one group belonging to CompileUnit (Variables etc that will be referenced in a single CU), the other group belonging to DwarfDebug (type system that can be shared across CUs).</div>
</div></div></div></blockquote><div><br></div></div><div>I don't believe that will be sufficient, but I could be wrong. Namespaces seem like a pretty solid example where a thing would need to be mapped in both a CU-specific and cross-CU way.<br>
<br>For types, currently there are cases where we insert member DIEs into type DIEs lazily (related to the vtable debug info optimization, wherein we emit only a declaration of a type when that type is dynamic (has a vtable) and the vtable isn't emitted in this translation unit).</div>
</div></div></div></blockquote></div><div>Do we have an existing testing case for this? I can try to extend it to multiple CUs to check what will happen.</div><div class="im"><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 class="gmail_extra"><div class="gmail_quote"><div> It might not be wrong to have us insert those member DIEs in an existing instance of the type in another compile_unit - but I'm hesitant to assume that's good and right.</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"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<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 class="gmail_extra"><div class="gmail_quote">
<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 class="gmail_extra"><div class="gmail_quote">
<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 class="gmail_extra"><div class="gmail_quote"><div>
<br>But I'm not sure how effective this would be - like I said, might need some thought/discussion to understand your solution & consider alternatives.</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>
Sometimes, when we try to add an attribute to a DIE, the DIE is not yet added<br>
to its owner yet, so we don't know whether we should use ref_addr or ref4.<br>
We create a worklist that will be processed during finalization to add<br>
attributes with the correct form (ref_addr or ref4).<br></blockquote><div><br></div></div><div>Do you have an example of this as a test case? (I really wish we had an easy way to see test coverage - so I could just tell at a glance whether the worklist code is being exercised by your test cases... oh well)</div>
</div></div></div></blockquote></div><div>Yes, we don't always immediately add a newly-created DIE to a parent. So it is possible to have DIEs without an owner when calling addDIEEntry.</div><div>One interesting example is:</div>
<div>getOrCreateTypeDIE --> addToContextOwner --> getOrCreateContextDIE (the original type DIE can be referenced during the construction of the context, and when that happens, the type DIE does not have an owner yet)</div>
</div></div></div></blockquote><div><br></div></div><div>But you know that the reference to that type DIE will be from within the same CU as the type itself - since it's a child. So you know the reference will be a ref4, no?</div>
</div></div></div></blockquote></div><div>For this case, yes, it is going to be ref4. But I am not sure whether we can say the same for all cases when a DIE is not added to an owner.</div><div>It also means we need to pass in an extra flag telling addDIEEntry that we are sure it is a ref4 and it is not an easy task to track the flag across the chain of</div>
<div>function calls: <span style="color:rgb(80,0,80)">getOrCreateTypeDIE --> addToContextOwner --> getOrCreateContextDIE --> ... --> addDIEEntry</span>.</div></div></div></div></blockquote><div><br></div></div>
<div>
Why would you need an extra flag? If we know that any time we come back around to the same entity that isn't already inserted (ie: the condition you already know because you use it to decide to add things to a work list) it must be a local reference, so you answer the question immediately rather than using a work list.<br>
</div></div></div></div></blockquote></div><div>How can we tell that we are coming back around to the same entity? Maybe I misunderstood something here.</div><div>When we see a DIE without an owner and add things to a work list, I don't think there is a guarantee that the DIE and the Entry belong to the same CU.</div>
<div><br></div><div>Thanks,</div><div>Manman</div><div class="im"><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 class="gmail_extra"><div class="gmail_quote"><div>
<br>I could be wrong, but I want to have a good reason to do it some other way not just "maybe sometimes this might not happen".<span><font color="#888888"><br><br>- David</font></span></div><div><div>
<div> </div></div></div></div></div></div>
</blockquote></div></div><br></div></div>
</blockquote></div><br></div>