<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Feb 10, 2014 at 4:42 PM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@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 class=""><br>
On Feb 10, 2014, at 16:14, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
<br>
> +Bob & Adrian who particularly care about this<br>
><br>
> I think the general consensus was to just bump the version number for each LLVM release (or private release - when any developer has one for which they need this).<br>
><br>
> So I think we essentially want to flag the tree as "we've broken compatibility" and whoever hits a release first (internal or LLVM official) should bump the version at that point and then the tree is "unbroken". Rather than bumping it for every change.<br>

><br>
</div>If we do this we need to document the official procedure somewhere. We would need to have a Debug info canary somewhere as the flag that current trunk is incompatible to any previously released version although the version number is the same and then anyone who cares may reset the flag and bump the version number (at any time?).<br>

<br>
We really need to make sure that it will not be easy to accidentally release two incompatible incarnations of LLVM with the same Debug info version number.<br>
<div class=""><br>
> We don't want to do this more often than necessary because it involves updating /all/ the debug info test cases.<br>
><br>
<br>
</div>We should also provide a script that recursively greps for<br>
  ![[0-9]]+ = metadata !{i32 2, metadata !"Dwarf Version", i32 [[0-9]]+}<br>
and bump the version. I guess it could also reset the canary.<br></blockquote><div><br></div><div>Yes, providing a script that does this seems like a fairly simple/good idea - it's really just a one-liner of find + sed (plus another line to reset the canary, as you say).</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Now that I think about it, I believe it would be better to provide only this script and run it every time someone breaks compatibility. </blockquote>
<div><br></div><div>That's going to be pretty churny - granted we don't break things often at the moment, but this does seem a bit painful, especially if you propose doing this in the same commit as the version break (because then code review becomes harder - finding the 2 test cases with meaningful changes amongst a field of version bumps?)</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Why? Suppose we have two changes on trunk that break compatibility: Change 1 sets the flag. Change 2 later doesn’t, because it’s already there, and there was no release in the meantime. When someone needs to cherry-pick only Change 2 into a release branch, because it fixes an important bug, there is no way to tell whether that branch should bump the version number or not.</blockquote>
<div><br></div><div>Not sure there's a great answer here, but we could have some means to touch the canary file each time debug info version is broken (even if it's already in a "broken" state). I'm not sure if you'd have trouble with this if you cherry picked Change 2, bumped the upstream version, cherry picked that, then later on integrated the whole branch - not realizing you needed another bump.<br>
<br>I think I see the problem you're dealing with now - if you can pick up arbitrary revisions you made need arbitrarily many version bumps and simply having a version bump after all breaks may not be sufficient for your needs.<br>
<br>I think this is a bit unfortunate - but not sure just how much it matters. Perhaps we can have a blessed "magic/any version" which we use for test cases, and just one test case to check the version handling?</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> (Obviously version numbers are only meaningful within the same “family” of releases.)<br>
<br>
base (V 100)<br>
 |<br>
change 1 (V101)<br>
 |<br>
change 2 (V102)<br>
 |<br>
ToT<br>
<br>
For private releases, one could use an expanded namespace:<br>
my_release1(base) (V 100100)<br>
my_release1.1(base+change2) (V 100101)<br>
<br>
my_release2(ToT) (V 200102)<br>
<span class="HOEnZb"><font color="#888888"><br>
<br>
<br>
-- adrian<br>
</font></span><div class="HOEnZb"><div class="h5">><br>
> On Mon, Feb 10, 2014 at 2:26 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br>
> Might want to increase the debug version number because of this.<br>
> Previously working code will start asserting/crashing (see PR18790).<br>
><br>
> -eric<br>
><br>
> On Mon, Feb 3, 2014 at 3:08 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> > Author: dblaikie<br>
> > Date: Mon Feb  3 17:08:54 2014<br>
> > New Revision: 200721<br>
> ><br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=200721&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=200721&view=rev</a><br>
> > Log:<br>
> > DIBuilder: simplify array generation to produce true zero-length arrays<br>
> ><br>
> > For some anachronistic reason we were producing {i32 0} for zero-length<br>
> > debug info arrays.<br>
> ><br>
> > (this change is paired with a Clang change and may cause temporary<br>
> > buildbot noise)<br>
> ><br>
> > Let's not.<br>
> ><br>
> > Modified:<br>
> >     llvm/trunk/lib/IR/DIBuilder.cpp<br>
> ><br>
> > Modified: llvm/trunk/lib/IR/DIBuilder.cpp<br>
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=200721&r1=200720&r2=200721&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=200721&r1=200720&r2=200721&view=diff</a><br>

> > ==============================================================================<br>
> > --- llvm/trunk/lib/IR/DIBuilder.cpp (original)<br>
> > +++ llvm/trunk/lib/IR/DIBuilder.cpp Mon Feb  3 17:08:54 2014<br>
> > @@ -902,10 +902,6 @@ DIBuilder::createForwardDecl(unsigned Ta<br>
> ><br>
> >  /// getOrCreateArray - Get a DIArray, create one if required.<br>
> >  DIArray DIBuilder::getOrCreateArray(ArrayRef<Value *> Elements) {<br>
> > -  if (Elements.empty()) {<br>
> > -    Value *Null = Constant::getNullValue(Type::getInt32Ty(VMContext));<br>
> > -    return DIArray(MDNode::get(VMContext, Null));<br>
> > -  }<br>
> >    return DIArray(MDNode::get(VMContext, Elements));<br>
> >  }<br>
> ><br>
> ><br>
> ><br>
> > _______________________________________________<br>
> > llvm-commits mailing list<br>
> > <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
<br>
</div></div></blockquote></div><br></div></div>