<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div></div><div><blockquote type="cite"><p dir="ltr">><br>
> > Any suggestion on how to generate a unique name for internal types<br>
> > defined within a lexical scope?<br>
> > int a(int i) {<br>
> > if (i) {<br>
> > struct A {<br>
> > …<br>
> > };<br>
> > }<br>
> > }<br>
><br>
> Yep. Walk up the scope appending the name of enclosing<br>
> functions/namespace/etc to the name.</p><p dir="ltr">We should have mangling support for a such names (as they can be used for template instantiations), plus adding the cu filename for anything with internal linkage.</p><p dir="ltr">><br>
> ><br>
> > The existing testing cases need major cleanup, they are not in verifiable<br>
> > states. As I mentioned earlier, there are 110 testing cases that failed to<br>
> > verify.<br>
> > It is hard to correctly update the testing cases since we don't have the<br>
> > source code to regenerate the debug info from.<br>
><br>
> Yep. This is why we started putting in the C code if we could.</p><p dir="ltr">Yes. We can't leave the test cases to rot. We must update them as the schema changes or we lose a lot of test coverage because it no longer tests what users are using.</p><p dir="ltr">If/when we implement a full verifier we will find lots of outdated/incomplete/broken tests and we will have to update or delete them. I suggest we do that incrementally, verifying new parts and updating tests cases go pass each time. Yes, this will be a lot of work.</p><p dir="ltr">><br>
> > The only way I can think of is to go back to the state before David's series<br>
> > of changes (assuming that state is correct, which I am not sure about), then<br>
> > re-apply the changes manually or using script.<br>
> > I think David submitted > 5 patches to change the format of the DI MDNodes.<br>
> ><br>
> > Before cleaning up the testing cases, I don't think it is a good idea to<br>
> > perform another series of update to the testing cases.</p><p dir="ltr">We can't let the test cases rot. If we need to add stronger verification now, then we need to update the existing tests to pass such verification.</p><p dir="ltr">> ><br>
><br>
> That's fair, but it seems like we don't need a lot of update. Just<br>
> about any format change is going to involve a lot of changing of debug<br>
> info test cases. It's unfortunate but true.<br>
><br>
> > If we want to go with the lookup option, we need to combine all above<br>
> > (additional code changes and testing cases update) in a single patch.<br>
> > I am not against the lookup option, but does not like submitting a big<br>
> > patch: 80K code change + 229k added testing cases + xxx existing testing<br>
> > cases.</p><p dir="ltr">Yeah, I don't see why this is true. Why can't we move one type field over at a time?</p><p dir="ltr">And if we need to add stronger verification (not sure why that need is happening now, but I'm pk if it is, we should do it as soon as we can) then, as I said, we should do so incrementally - verifying one feature, updating testsm etc.</p><p dir="ltr">> ><br>
><br>
> Gah. You sure now after your patch of earlier?<br>
><br>
> > Let me know your thoughts on how to move this forward.<br>
> ><br>
><br>
> Hope this helps.<br>
><br>
> -eric<br>
><br>
> > Thanks,<br>
> > Manman<br>
> ><br>
> > On Jul 15, 2013, at 10:13 PM, David Blaikie wrote:<br>
> ><br>
> ><br>
> > On Jul 15, 2013 10:06 PM, "Eric Christopher" <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br>
> >><br>
> >> As a note, pretty sick right now. I'll get to this shortly. As a quick<br>
> >> look I think we don't want to pass in the map, but rather look it up<br>
> >> in the uses?<br>
> ><br>
> > Manman- just to expand on this, since Eric and I have tossed the idea around<br>
> > a bit...<br>
> ><br>
> > Rather than having foo.getThing(map) we'd have something like<br>
> > lookup(foo.getThing()) - we'd need a return type from getThing that<br>
> > represent as the type identifier (the thing we keep calling a 'hash' but it<br>
> > isn't), we could just use stringref or something more specific.<br>
> ><br>
> > Does that make sense? What do you think (we didn't really have a strong<br>
> > feeling, but I understand/can agree wit Eric erring this way)<br>
> ><br>
> > As we discussed before, we shouldn't need conditional lookup (so we<br>
> > shouldn't be in a state where 'getThing' returns something that might be a<br>
> > type identifier or might be the type itself). We should just migrate one<br>
> > field at a time (or a bunch or all of them, but not switching the consuming<br>
> > code before the production code, otherwise were just adding a bunch of<br>
> > dead/untested code)<br>
> ><br>
> >><br>
> >> -eric<br>
> >><br>
> >> On Fri, Jul 12, 2013 at 3:13 PM, Manman Ren <<a href="mailto:mren@apple.com">mren@apple.com</a>> wrote:<br>
> >> ><br>
> >> > Thanks,<br>
> >> > Manman<br>
> >> ><br>
> >> > On Jul 12, 2013, at 2:44 PM, Eric Christopher wrote:<br>
> >> ><br>
> >> >> Getting to it shortly, week has been roughly crazy.<br>
> >> >><br>
> >> >> -eric<br>
> >> >><br>
> >> >> On Thu, Jul 11, 2013 at 9:24 PM, Manman Ren <<a href="mailto:mren@apple.com">mren@apple.com</a>> wrote:<br>
> >> >>><br>
> >> >>> Private ping :)<br>
> >> >>><br>
> >> >>> Did you get a chance to look at the patch?<br>
> >> >>><br>
> >> >>> Things on my list:<br>
> >> >>> 1> refactor a helper function to query module flag<br>
> >> >>> 2> usage of getDICompositeType on dragonegg<br>
> >> >>> 3> try to use assert in r185847<br>
> >> >>><br>
> >> >>> Thanks,<br>
> >> >>> Manman<br>
> >> >>><br>
> >> >>> On Jul 9, 2013, at 9:48 AM, Manman Ren wrote:<br>
> >> >>><br>
> >> >>>><br>
> >> >>>> Modify access functions that can return a DIType to take an extra map<br>
> >> >>>> argument.<br>
> >> >>>> Add helper function<br>
> >> >>>> + void collectDITypeHashMap(const NamedMDNode *CU_Nodes,<br>
> >> >>>> DITypeHashMap &Map);<br>
> >> >>>> to update DITypeHashMap by going through RetainedTypes of each CU.<br>
> >> >>>> Testing cases are added to verify llvm-link can correctly unique<br>
> >> >>>> types if the input .ll files use string instead of DIType.<br>
> >> >>>><br>
> >> >>>> To make sure we can dump MDNode comments even without a<br>
> >> >>>> DITypeHashMap, isDebugInfo is used instead of<br>
> >> >>>> Verify in WriteMDNodeComment. Neither is accurate for checking<br>
> >> >>>> whether MDNode is indeed debug info.<br>
> >> >>>><br>
> >> >>>> Thanks,<br>
> >> >>>> Manman<br>
> >> >>>><br>
> >> >>>> <type_unique_di.patch><type_unique_test.patch><br>
> >> >>><br>
> >> ><br>
> ><br>
> ><br>
</p>
</blockquote></div><br></body></html>