<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div></div><div><br></div><div>We are calling Verify to make sure a MDNode is indeed a specific DI node.</div><div><br></div><div><br><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>If we're using it to differentiate nodes (which I think I've done in<br>one or two places) that's probably a bit dodgy & we should do<br>something more explicit (based on tag values, etc).<br><br>I think the trickiest use of it (& even then it's not quite<br>sufficient) is in the assembly comment printing stuff - I'm not sure<br>there's a good alternative there. Treating any metadata that happens<br>to have an integer first operand that happens to be a tag value is...<br>unreliable at best. Not sure what to do about that.<br></div></blockquote>Same feeling here. But we can only parse the content of the MDNode to know whether it is a DI node and</div><div>if yes, what type of DI node it is.</div><div><br><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><blockquote type="cite">If you prefer to communicate via lists, let me know.<br></blockquote><br>Generally, yes. Discussion should happen in the open unless there's a<br>need for it to be private.<br><br><blockquote type="cite">As to the MDNode interface, I think debug info is the biggest user, other usage is quite simple, such as TBAA.<br></blockquote><br>I'm not sure what you're referring to here.<br></div></blockquote><div><br></div>I am referring to the fact nobody else had comments on our proposal of modifying MDNode :]</div><div>That probably means we are the ones who care the most about MDNode?</div><div><br></div><div>Thanks,</div><div>Manman</div><div><br><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><blockquote type="cite"><br>Thanks,<br>Manman<br><br>On Jun 21, 2013, at 10:01 PM, Manman Ren wrote:<br><br><blockquote type="cite"><br>Some scoping for option a:<br><br>There are a few Verify functions<br>DISubprogram::Verify<br>DICompositeType::Verify<br>DIType::Verify<br>that try to access the context link:<br>if (getContext() && !getContext().Verify())<br> return false;<br><br>And the Verify functions are called from 10+ files.<br><br>If we are oaky with not calling getContext().Verify(), option a) appears much better to me.<br>For printing | debugging purpose, we agree that not being able to trace the pointer is okay.<br><br>Let me know your thoughts,<br>Manman<br><br>On Jun 21, 2013, at 5:01 PM, Eric Christopher wrote:<br><br><blockquote type="cite">On Fri, Jun 21, 2013 at 4:59 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br><blockquote type="cite">On Fri, Jun 21, 2013 at 4:50 PM, Manman Ren <<a href="mailto:mren@apple.com">mren@apple.com</a>> wrote:<br><blockquote type="cite"><br>On Jun 21, 2013, at 4:41 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br><br>The reason I want a flag is to avoid the need to update the existing testing<br>cases while this is a work in progress.<br>I believe migrating one field means updating the existing testing cases?<br><br><br>It would, yes - and that's how you'd get coverage to know your change<br>was stable. You'll have to update all the tests eventually anyway &<br>doing so incrementally isn't substantially more expensive in my<br>experience. Perhaps there's something unique to this migration that<br>would make it so?<br><br><br>I am going to update the existing testing cases locally to have the test<br>coverage, but I don't want to check in the changes often.<br>Once we turn the flag on by default and remove the other path, I will submit<br>the changes on the testing cases.<br></blockquote><br>I'd rather skip the flag & have the test coverage in the open along<br>with the changes. Having changes without testing is bad. I don't see a<br>benefit to you keeping the test coverage locally.<br><br><blockquote type="cite">Another point to have a flag is to check in the patches in steps: DIBuilder<br>changes, changes related to the map, and DwarfDebug changes.<br>Without the flag, when I migrate the first field, I have to make sure it<br>works from frontend all the way to the backend.<br></blockquote><br>True enough, but I don't think that's a great drawback.<br><br>Though technically, if you really wanted, you could start at the<br>backend & make the implementation conditional (check the type of the<br>field, if it's an MDNode - use that, if it's a string, do the lookup)<br>- implement that, then port DIBuilder, then remove the conditionality.<br>Though I don't like it a lot & I think the changes will be small<br>enough that frontend to backend will be still be understandable<br>patches.<br><br></blockquote><br>All of this.<br><br>-eric<br></blockquote><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</blockquote></blockquote></div></blockquote></div><br></body></html>