<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Nov 21, 2013 at 11:06 AM, 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><div class="h5">On Thu, Nov 21, 2013 at 10:57 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: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><div>On Thu, Nov 21, 2013 at 10:38 AM, 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><div>On Tue, Nov 19, 2013 at 8:47 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 Tue, Nov 19, 2013 at 5:26 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><div>On Mon, Nov 18, 2013 at 11:00 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.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"><div class="gmail_extra"><div><br>
<div class="gmail_quote">On Mon, Nov 18, 2013 at 10:55 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>It depends a bit, also, on what kind of guarantees we need to offer. If the guarantee when reading IR from disk is "will not crash" then there's nothing for it but to run full debug info verification. <br>
<br>On the other hand, if we can assume that some specific metadata implies the correctness of some other metadata, then all we need to do is check a known debug info version number. If it's the current number, do nothing (assume the rest of the debug info is up to date and well formed), otherwise do all the work to find the debug info and dump it (no need to verify it - we just have to do the work to find it, so we don't go following bad links later on - that should be as easy as dropping the <a href="http://llvm.dbg.cu" target="_blank">llvm.dbg.cu</a> named node and removing all debug intrinsics and the instruction metadata line references). But this latter scheme isn't robust against arbitrary metadata (that could, coincidentally, have the right version number and arbitrary metadata that breaks all our debug info metadata assumptions)<br>
<br>If the latter is sufficient for everyone's needs/principles, great.</div></blockquote></div><br></div>This makes sense to me, but I see Eric's fundamental concern with upgrading test cases.</div><div class="gmail_extra">
<br></div><div class="gmail_extra"><br></div><div class="gmail_extra">One other possible idea: we could artificially force coarseness on metadata while things are still iterating rapidly by just having a version number that rotates every "release". (Even non-open-source-releases could be handled by letting anyone, any time they need to rev it, update what the current version is and update every test case to reference that version.) If the version is nicely factored, it should at least be an obvious diff if still huge.</div>
</div></blockquote><div><br></div></div></div><div>Adding a debug info version number and ignoring the debug info MDNodes when version does not match makes sense. And changing the version number with every release sounds reasonable.</div>
<div><br></div><div>One implementation is</div><div>1> completely get rid of version number in the tag</div><div> LLVMDebugVersion is only used in two places and we should be able to remove it.<br></div><div> include/llvm/DebugInfo.h: return getUnsignedField(0) & ~LLVMDebugVersionMask;</div>
<div> lib/IR/DIBuilder.cpp: assert((Tag & LLVMDebugVersionMask) == 0 &&</div><div> lib/IR/DIBuilder.cpp: return ConstantInt::get(Type::getInt32Ty(VMContext), Tag | LLVMDebugVersion);</div><div> I remember there was something that blocks David from completely getting rid of version number, but it seems the blocker is gone now.</div>
<div> This can be done separately as well.</div><div><br></div><div>2> introduce a debug info version in module flags similar to "dwarf version" module flag</div><div> LTO can correctly handle the module flag during linking and we only need to change one line in the debug info testing cases when we update debug info version number.</div>
<div> One issue is that we don't have a mode that emits a warning and outputs the largest value (we have a mode that emits a warning and outputs the value from the first module, but we can definitely add another mode if necessary)</div>
<div><br></div><div>3> when to drop the debug info MDNodes?</div><div> We could do this in the bit code loader but it seems a violation of layers. One possibility is to run a module pass right after loading the bit code, to remove debug intrinsics and debug tags on instructions.</div>
</div></div></div></blockquote><div><br></div></div><div>We have an existing pass StripSymbols that can strip debug info in the module.</div>
<div>So it is just a matter of adding that pass after bit code loader if the debug info version number does not match.</div></div></div></div></blockquote><div><br></div></div></div><div>Can I assume no objection to the approach? :)</div>
<div>Bill, are you okay with adding another mode to module flags? i.e. emits a warning and outputs the largest value.</div></div></div></div></blockquote><div><br></div></div></div><div>How will this work as a module flag, though? If the flag gets merged and maximized, how will we know which compile units have out of date debug info metadata and drop them?</div>
</div></div></div></blockquote><div> </div></div></div><div>The dropping part comes from item 3 above: adding StripSymbols pass right after bit code loader if the debug info version number does not match.</div></div></div>
</div></blockquote><div><br></div><div>(what Eric said, but also)<br><br>I'm just not really understanding how you choose which things to strip in (3). If you've already merged IR modules together and the debug info version is bumped to the maximum - if we had an old and a new module, the resulting version would be 'new' and we wouldn't know we still had some old data in there we needed to remove, would we? (and even if we did, we wouldn't know which of the compile units we needed to drop and which intrinsics we needed to strip, etc - since some was new and good and some was old and busted)</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>Item 3 is shared between non-LTO and LTO. For LTO, we want the debug info version number gets merged and maximized.</div>
<div><br></div><div>Thanks</div><span class="HOEnZb"><font color="#888888"><div>Manman</div></font></span><div class="im"><div><br></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><span><font color="#888888"><br>
<br>- David</font></span></div><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><br></div><div>Thanks,</div>
<div><div>Manman</div><div><br></div><div><div dir="ltr"><div class="gmail_extra">
<div class="gmail_quote"><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><br></div><div>4> how to report warning?</div><div> Is errs() << "WARNING: " good enough? BTW this is how we emit warning when linking module flags.</div><div><br></div><div>Any objection to the above? Any other suggestion?</div>
<div><br></div><div>Thanks,</div><div>Manman</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>
<br>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu" target="_blank">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
<br></blockquote></div></div><br></div></div>
</blockquote></div></div><br></div></div>
</div></div></div><br></div></div>
</blockquote></div></div><br></div></div>
</blockquote></div></div><br></div></div>
</blockquote></div><br></div></div>