<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">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 class="im"><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>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> </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 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>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>
Other debuggers should work with ref_addr.</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 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>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 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>
<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>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><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>Oops, I moved the mapping from CompileUnit to DwarfDebug. Are you referring to "the namespace alias" by "some cases"? </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 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>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>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> </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 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>
<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>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> </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 class=""><font color="#888888"><br><br>- David</font></span></div><div><div class="h5">
<div> </div></div></div></div></div></div>
</blockquote></div><br></div></div>