[llvm] r200721 - DIBuilder: simplify array generation to produce true zero-length arrays

[Eric Christopher]

On Mon, Feb 10, 2014 at 4:42 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
> On Feb 10, 2014, at 16:14, David Blaikie <dblaikie at gmail.com> wrote:
>
>> +Bob & Adrian who particularly care about this
>>
>> 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).
>>
>> 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.
>>
> 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?).
>

... I suppose we could. But largely it's a documentation and not a code issue.

> 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.
>

We can probably just assume it needs to bump every time we cut a
release branch :p

That said we don't want to update all of the testcases all the time.
It's just a somewhat annoying churn.

>> We don't want to do this more often than necessary because it involves updating /all/ the debug info test cases.
>>
>
> We should also provide a script that recursively greps for
>   ![[0-9]]+ = metadata !{i32 2, metadata !"Dwarf Version", i32 [[0-9]]+}
> and bump the version. I guess it could also reset the canary.
>
>
> 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. 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. (Obviously version numbers are only meaningful within the same "family" of releases.)
>

I disagree here. If you're cherry-picking then you'd better know what
the change does - including breaking compatibility and can update the
default value yourself.

-eric

> base (V 100)
>  |
> change 1 (V101)
>  |
> change 2 (V102)
>  |
> ToT
>
> For private releases, one could use an expanded namespace:
> my_release1(base) (V 100100)
> my_release1.1(base+change2) (V 100101)
>
> my_release2(ToT) (V 200102)
>
>
>
> -- adrian
>>
>> On Mon, Feb 10, 2014 at 2:26 PM, Eric Christopher <echristo at gmail.com> wrote:
>> Might want to increase the debug version number because of this.
>> Previously working code will start asserting/crashing (see PR18790).
>>
>> -eric
>>
>> On Mon, Feb 3, 2014 at 3:08 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> > Author: dblaikie
>> > Date: Mon Feb  3 17:08:54 2014
>> > New Revision: 200721
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=200721&view=rev
>> > Log:
>> > DIBuilder: simplify array generation to produce true zero-length arrays
>> >
>> > For some anachronistic reason we were producing {i32 0} for zero-length
>> > debug info arrays.
>> >
>> > (this change is paired with a Clang change and may cause temporary
>> > buildbot noise)
>> >
>> > Let's not.
>> >
>> > Modified:
>> >     llvm/trunk/lib/IR/DIBuilder.cpp
>> >
>> > Modified: llvm/trunk/lib/IR/DIBuilder.cpp
>> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=200721&r1=200720&r2=200721&view=diff
>> > ==============================================================================
>> > --- llvm/trunk/lib/IR/DIBuilder.cpp (original)
>> > +++ llvm/trunk/lib/IR/DIBuilder.cpp Mon Feb  3 17:08:54 2014
>> > @@ -902,10 +902,6 @@ DIBuilder::createForwardDecl(unsigned Ta
>> >
>> >  /// getOrCreateArray - Get a DIArray, create one if required.
>> >  DIArray DIBuilder::getOrCreateArray(ArrayRef<Value *> Elements) {
>> > -  if (Elements.empty()) {
>> > -    Value *Null = Constant::getNullValue(Type::getInt32Ty(VMContext));
>> > -    return DIArray(MDNode::get(VMContext, Null));
>> > -  }
>> >    return DIArray(MDNode::get(VMContext, Elements));
>> >  }
>> >
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>