<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Oct 2, 2013 at 6:45 PM, Manman Ren <span dir="ltr"><<a href="mailto:mren@apple.com" target="_blank">mren@apple.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="auto"><div><div class="h5"><div><br></div><div><br>On Oct 2, 2013, at 4:13 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
<br></div><blockquote type="cite"><div><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Oct 2, 2013 at 4:05 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>On Wed, Oct 2, 2013 at 10:42 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 Tue, Oct 1, 2013 at 7:05 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: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>I am now a little confused about the mapping issue.</div>
<div>The patch replaces every getDIE of a Type, a SP, and a Static Member with DwarfDebug.getType|SP|StaticMemberDie.</div>
<div>
<br></div><div>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><br></div><div>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><br></div><div>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>have multiple DIEs for the MDNode.</div><div><br></div><div>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></blockquote><div><br></div></div><div>OK, I think I'm understanding you here. Just throwing around thoughts, nothing terribly deeply analyzed yet, bu it might be easier to understand if we simply had CompileUnit::getDIE have a list of tags for which it would delegate to the cross-CU mapping. Then it'd be simply one mapping in DwarfDebug used from one place (getDIE) and would be hidden from all callers, keeping the rest of the code fairly consistent/simple... just a thought, at least.<br>
</div></div></div></div></blockquote><div><br></div></div><div>Sounds reasonable, I will work on that.</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>But specifically you're just interested in sharing types between CUs, yes? The sharing of Subprograms is just really for sharing them when they're members of a type, yes?</div></div></div></div></blockquote></div>
<div>
Yes. </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 suppose subprograms could appear in multiple CUs if they were inline in a header and used in multiple CUs)<br>
<br>Would it perhaps be easier to remove this patch and just use type units that I'm working on implementing at the moment?<br></div></div></div></div></blockquote><div><br></div><div><br></div></div><div><div>This patch does two things:</div>
<div>1> move the map from CompileUnit to DwarfDebug</div><div>2> correctly use ref_addr if necessary (by using a worklist)</div></div><div><br></div><div>There are some functionality overlapping between this patch and the type units, but there are certainly differences as well.<br>
</div></div></div></div></blockquote><div><br></div><div>I'm not so interested in a comparison of their implementation as I am in a comparison of their observable result to debug info.</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>
</div><div>Both need to have a map in DwarfDebug that maps type MDNodes to DIE(s), in your proposed patch,<br></div><div>DenseMap<const MDNode *, std::pair<uint64_t, SmallVectorImpl<DIE*>* > > TypeUnits</div>
<div> --> I am not quite sure why we need the pair, can you add some comments in your proposed patch? :)</div></div></div></div></blockquote><div><br></div><div>It was mostly just out as a straw man. The pair is a semi-descriminated union, the vector is your 'work list', I believe. While we're generating the type unit for some type T, the pair will have a non-null vector and an irrelevant uint64 (because we haven't finished generating the type, so we can't compute the hash for it). When a non-null vector is seen, DIEs that should be skeletons for the type unit are added to the vector. When we finish a type unit, we attach the signature to all those DIEs. From then on, whenever we're looking to build a skeleton DIE for a type, we have the signature in the uint64_t (because the vector pointer is null, we know the signature is valid) and we just attach it immediately.</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>In my patch, we have a DenseMap<const MDNode *, DIE*> MDTypeNodeToDieMap</div>
<div><br></div><div>
With type units, the across-CU references will be replaced with references to type units.</div><div>With this patch, we use ref_addr.</div><div><br></div><div>For LTO's purpose, we don't need to calculate the hash since all we need is to make sure we only generate a single DIE for a given type MDNode.</div>
<div>Calculating the hash costs time :)</div></div></div></div></blockquote><div><br></div><div>Enough to matter? I wonder.<br></div></div></div></div></div></blockquote></div></div>I wonder about that too. Can we do any measurement now?</div>
</blockquote><div><br>Ideally, you'd have provided, in the commit message, relevant numbers that justify your change (see, for example, the sort of stats I gave with debug info size improvements I made a few months ago such as the vtable-driven type info emission). If the targets you're testing against are private, a good way to demonstrate value to the community is if you can show the value for Clang (or other portions of LLVM) on Clang. <br>
<br>eg: compiling Clang with Clang produces debug info that's X% smaller after this change than before<br><br>(of course, for precision, the 2nd Clang build should use the same source between the two comparisons. The way I usually do this is sync my changes in, build clang, swap my changes out (so I have pristine ToT Clang) then run 3 builds - build Clang with the just-built Clang, rebuild my release Clang, then build Clang again with the newly built Clang. I at least put the 2nd and 4th builds of Clang into separate places so I can, as a final step, gather data. One simple way I've been gathering data is to just sum the size of the '.dwo' files in a -gsplit-dwarf build - this would be a reasonable comparison for your work, as I don't think your change would affect the non-dwo sections anyway. But I do also have a script that can find all the .o and .dwo files and create a CSV file full of the size of each debug_ section. This gives a more precise breakdown but takes a bit longer to gather.<br>
<br>Also, be careful that you do clean builds, and you might have to manually nuke the .dwo files if you use fission as the CMake build, at least, doesn't seem to blow them away)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="auto"><div class="im"><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>Of course type units will duplicate more things - they'll have the indirection of the type skeleton (a structure/class_type with a signature attribute), and duplicate member functions for implicit members (nested types, member template instantiations, and implicit special member functions) and function definitions. But I don't know just how significant that win will be/whether it'll be worth doing when we get most of the win from type units - maybe the extra won't be enough to justify the added complexity. (indeed this commit didn't come with any numbers about how much it helps)</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 would like to revisit this (maybe revert this) later on when type units are working.<br>
</div></div></div></div></blockquote><div><br></div><div>I'd rather revert this before type units (& then revisit this afterwards) as it may complicate the type unit work.</div></div></div></div></div></blockquote>
</div>What kind of complication do you see ?</div></blockquote><div><br></div><div>Having extra caching up in the cross-CU code (in DwarfDebug) is going to make it a bit trickier/more complicated to implement type units, as they're another kind/way of reusing types at that layer.<br>
<br>I realize this is sort of a "who goes first" question, but I think there's good reason for type units to go first. They're a well-known and desired DWARF feature that, I suspect, will have most of the benefits of your change. Having type units implemented first, separately from your change, will let us see how it looks/what the best way to implement it is. Then we can consider a change like yours, discuss its design, its value (what % gain does it give us over type units) and whether the increased complexity is worth the gain.<br>
<br>As a general note - it'd be nice to have more open/continuous discussion about work like this in the future. This isn't to say that I'm looking for an opportunity to veto or otherwise vet changes you want to make, but ultimately we want to all be on board with the direction the codebase is going so it's helpful to come to some consensus on such things, perhaps toss around alternative ideas, etc. (for example, the way I sent out the type unit patch for review - though admittedly I could've done a better job of discussing that in public earlier as I was looking into it, just to make sure everyone knew where that was going)<br>
<br>- David</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto"><div><div><br></div><div>Manman<br><div><div><div class="h5"><br><blockquote type="cite">
<div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>I think the extra benefit (using direct ref_addr, rather than signatures) of this approach over type units may be implementable via an incremental improvement to type units.</div>
</div></div></div></div></blockquote></div></div><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<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><div class="h5"><div><div><br></div>
<div><blockquote class="gmail_quote" style="font-family:arial,sans-serif;font-size:13px;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>+ /// Similar to getCompileUnit, returns null when DIE is not added to an<br>+ /// owner yet.<br>+ DIE *checkCompileUnit();<br></div></blockquote><div><br></div></div><div>Why is this not just "getParent() == NULL"? Why do you need a walk for the DIE?</div>
</div></div></div></blockquote><div style="font-family:arial,sans-serif;font-size:13px">Yeah, that should work.</div></div></div><div style="font-family:arial,sans-serif;font-size:13px"><-- It is possible that a DIE is added to a parent but the parent is not yet added to an owner yet, so we still have to walk along the parent chain.</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div>Thanks,</div><div>Manman</div></div></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">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><div><br>While I have discussed with Eric the idea that DIEs could be referenced cross-CU, I'm a bit hesitant to put this sort of thing in without further thought/consideration, and it seems like type units are a more general purpose solution to this and other issues anyway. (though they don't immediately address the inline non-member function example I mentioned above, those would still be duplicated across CUs - so if that's really important we might have to do something like this anyway)</div>
</div></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"><div dir="ltr">
<div><br></div><div>Thanks,</div><div>Manman</div></div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">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>
</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"><br><div class="gmail_extra"><br>
<br><div class="gmail_quote"><div><div class="h5">
<div>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><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></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 class="h5"><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></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 whewhy we need two mappings for one MDNode.</div>
</div></div></div></blockquote><div><br></div></div><div><div class="h5"><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></div></div></blockquote></div><div><div class="h5"><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><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><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><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><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></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></div></blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br>
<br>
<br>
<br>
<br>
<br>
<br>
div>
</div></blockquote><div class="im"><blockquote type="cite"><div><span>_______________________________________________</span><br><span>llvm-commits mailing list</span><br><span><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a></span><br>
<span><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></span><br></div></blockquote></div></div></div></div></div>
</blockquote></div><br></div></div>