llvm dev - Jun 2014 - [LLVMdev] Clarification on the backward compatibility promises

On 17 June 2014 16:07, Anton Korobeynikov <anton at korobeynikov.info>
wrote:
> Hi Rafael,
>
>> Do others agree that this is the case or at least that this would be a
>> reasonable balance?
> IMO it's easier to be compatible on .ll level, no?
That is not my experience with the bitcode format. The way the API is
structured makes it really easy to support backwards compatibility.

It also seems a lot more valuable from an user perspective to support
reading old .bc files. It means they can keep a library with IR for an
entire major LLVM release for example.
>In case of binary
> IR it's really easy to make incompatible changes. After all there are
> no tests on IR binary compatibility, however the whole regression
> testsuite can be viewed as a big test for .ll compatibility.
We do have tests that are done by checking in old versions of bitcode
files. We didn't use to be good about it, but I think we are now
fairly systematic about it any time we change the format.
> There are two more points here:
>
> 1. Actually we had much stronger policies wrt the bitcode
> compatibility in minor releases. Something like x.y should be able to
> read stuff from x.y-1, but x.y+2 is allowed not to read stuff there,
> so the proper path is transition x.y-1 => x.y => x.y+2. Am I right?
That doesn't match what we have in trunk right now.  For example, we
changed how inline asm is stored in r163185 (Sep 5 2012), but we still
support reading the old one. This is one of the cases where we have a
FIXME about 4.0.
> 2. Metadata compatibility. We already had precedence of introducing
> incompatible changes into metadata format in the past within release.
> Should we use relaxes rules for metadata compatibility?
I think we have a special case for debug metadata (and should document
that), but otherwise I think we should hold metadata to the same
standard as the rest of the IR.
>> In any case, we should probably document whatever we decide. Where
>> should that go? Sean suggested docs/DeveloperPolicy.html. Is everyone
>> OK with that?
> +1
Cheers,
Rafael

>> 2. Metadata compatibility. We already had precedence of introducing
>> incompatible changes into metadata format in the past within release.
>> Should we use relaxes rules for metadata compatibility?
>
> I think we have a special case for debug metadata (and should document
> that), but otherwise I think we should hold metadata to the same
> standard as the rest of the IR.
>
The idea with metadata is that it can be removed and everything still
works. I'm definitely not ready to lock down the debug metadata format
and I really don't think we should for any of the other uses since
stripping already works. (Note, I don't consider function attributes
etc as metadata)

-eric

>> I think we have a special case for debug metadata (and should document
>> that), but otherwise I think we should hold metadata to the same
>> standard as the rest of the IR.
>>
>
> The idea with metadata is that it can be removed and everything still
> works. I'm definitely not ready to lock down the debug metadata format
> and I really don't think we should for any of the other uses since
> stripping already works. (Note, I don't consider function attributes
> etc as metadata)
I think we mean the same thing in the end. Since metadata can be
dropped, one particular way in which upgrade can be done for an old
format is to drop it. This would miss any optimization that the old
format was intended to allow, but we don't promise to keep
optimizations.

What would not be OK is for us to change the format (for example, make
range take a closed interval) and start miscompiling IR that had the
old format in it.

Debug metadata is special only in that we accept more than a missed
optimization when it is dropped, and I agree it should remain like
that for now.

Cheers,
Rafael

On Tue, Jun 17, 2014 at 2:24 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:
> On 17 June 2014 16:07, Anton Korobeynikov <anton at
korobeynikov.info> wrote:
> > Hi Rafael,
> >
> >> Do others agree that this is the case or at least that this would
be a
> >> reasonable balance?
> > IMO it's easier to be compatible on .ll level, no?
>
> That is not my experience with the bitcode format. The way the API is
> structured makes it really easy to support backwards compatibility.
>
Could you elaborate more on this?

My anecdotal experience is that the .ll is more stable. I remember last
summer that in multiple situations passing the old .bc (from the 3.1-based
or 3.2-based SCE compiler IIRC) to trunk would cause it to barf, while
passing the .ll file would not barf. I don't think I would be able to
reproduce this without a lot of work, so I'm just leaving this as an
anecdote.

-- Sean Silva


>
> It also seems a lot more valuable from an user perspective to support
> reading old .bc files. It means they can keep a library with IR for an
> entire major LLVM release for example.
>
> >In case of binary
> > IR it's really easy to make incompatible changes. After all there
are
> > no tests on IR binary compatibility, however the whole regression
> > testsuite can be viewed as a big test for .ll compatibility.
>
> We do have tests that are done by checking in old versions of bitcode
> files. We didn't use to be good about it, but I think we are now
> fairly systematic about it any time we change the format.
>
> > There are two more points here:
> >
> > 1. Actually we had much stronger policies wrt the bitcode
> > compatibility in minor releases. Something like x.y should be able to
> > read stuff from x.y-1, but x.y+2 is allowed not to read stuff there,
> > so the proper path is transition x.y-1 => x.y => x.y+2. Am I
right?
>
> That doesn't match what we have in trunk right now.  For example, we
> changed how inline asm is stored in r163185 (Sep 5 2012), but we still
> support reading the old one. This is one of the cases where we have a
> FIXME about 4.0.
>
> > 2. Metadata compatibility. We already had precedence of introducing
> > incompatible changes into metadata format in the past within release.
> > Should we use relaxes rules for metadata compatibility?
>
> I think we have a special case for debug metadata (and should document
> that), but otherwise I think we should hold metadata to the same
> standard as the rest of the IR.
>
> >> In any case, we should probably document whatever we decide. Where
> >> should that go? Sean suggested docs/DeveloperPolicy.html. Is
everyone
> >> OK with that?
> > +1
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140618/cd5907f9/attachment.html>

>> That is not my experience with the bitcode format. The way the API is
>> structured makes it really easy to support backwards compatibility.
>
>
> Could you elaborate more on this?
>
> My anecdotal experience is that the .ll is more stable. I remember last
> summer that in multiple situations passing the old .bc (from the 3.1-based
> or 3.2-based SCE compiler IIRC) to trunk would cause it to barf, while
> passing the .ll file would not barf. I don't think I would be able to
> reproduce this without a lot of work, so I'm just leaving this as an
> anecdote.
Also anecdote, but since each "item" is output as a variable length
record, it is easy to append new members to the end. When we want to
make a more fundamental change, we can add a new enum value (like we
did for assembly).

Cheers,
Rafael

On 18 June 2014 17:10, Sean Silva <chisophugis at gmail.com>
wrote:
>> >> Do others agree that this is the case or at least that this
would be a
>> >> reasonable balance?
>> > IMO it's easier to be compatible on .ll level, no?
>>
>> That is not my experience with the bitcode format. The way the API is
>> structured makes it really easy to support backwards compatibility.
>
> Could you elaborate more on this?
>
> My anecdotal experience is that the .ll is more stable. I remember last
> summer that in multiple situations passing the old .bc (from the 3.1-based
> or 3.2-based SCE compiler IIRC) to trunk would cause it to barf, while
> passing the .ll file would not barf. I don't think I would be able to
> reproduce this without a lot of work, so I'm just leaving this as an
> anecdote.
Well, I can only speak for myself, but in my two recent IR changes
(cmpxchg failure orderings and cmpxchg weak"), I preserved bitcode
compatiblity but not .ll. In both cases I was adding extra information
to the instruction, which meant the bitcode reading section just had
to insert a sensible default if that field wasn't present.

In the first case, I could have kept IR compatibility, but the second
(r210903) would have been rather difficult. It would involve examining
all uses of the cmpxchg to find out whether the type expected was
compatible with the new or the old version.

Cheers.

Tim.

> On Jun 17, 2014, at 1:24 PM, Rafael Espíndola <rafael.espindola at
gmail.com> wrote:
> 
> On 17 June 2014 16:07, Anton Korobeynikov <anton at
korobeynikov.info> wrote:
>> Hi Rafael,
>> 
>>> Do others agree that this is the case or at least that this would
be a
>>> reasonable balance?
>> IMO it's easier to be compatible on .ll level, no?
> 
> That is not my experience with the bitcode format. The way the API is
> structured makes it really easy to support backwards compatibility.
> 
> It also seems a lot more valuable from an user perspective to support
> reading old .bc files. It means they can keep a library with IR for an
> entire major LLVM release for example.
> 
>> In case of binary
>> IR it's really easy to make incompatible changes. After all there
are
>> no tests on IR binary compatibility, however the whole regression
>> testsuite can be viewed as a big test for .ll compatibility.
> 
> We do have tests that are done by checking in old versions of bitcode
> files. We didn't use to be good about it, but I think we are now
> fairly systematic about it any time we change the format.
> 
>> There are two more points here:
>> 
>> 1. Actually we had much stronger policies wrt the bitcode
>> compatibility in minor releases. Something like x.y should be able to
>> read stuff from x.y-1, but x.y+2 is allowed not to read stuff there,
>> so the proper path is transition x.y-1 => x.y => x.y+2. Am I
right?
> 
> That doesn't match what we have in trunk right now.  For example, we
> changed how inline asm is stored in r163185 (Sep 5 2012), but we still
> support reading the old one. This is one of the cases where we have a
> FIXME about 4.0.
My understanding is newer version of LLVM should be able read older version with
the same major release. And the .0 of the new major release must be able to read
the bitcode of the previous major release. I think this is the right policy. We
haven’t done a good job enforcing the policy, but we should.
> 
>> 2. Metadata compatibility. We already had precedence of introducing
>> incompatible changes into metadata format in the past within release.
>> Should we use relaxes rules for metadata compatibility?
> 
> I think we have a special case for debug metadata (and should document
> that), but otherwise I think we should hold metadata to the same
> standard as the rest of the IR.
I agree.

Evan
> 
>>> In any case, we should probably document whatever we decide. Where
>>> should that go? Sean suggested docs/DeveloperPolicy.html. Is
everyone
>>> OK with that?
>> +1
> 
> Cheers,
> Rafael
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

> On Jun 17, 2014, at 2:10 PM, Eric Christopher <echristo at gmail.com>
wrote:
> 
>>> 2. Metadata compatibility. We already had precedence of introducing
>>> incompatible changes into metadata format in the past within
release.
>>> Should we use relaxes rules for metadata compatibility?
>> 
>> I think we have a special case for debug metadata (and should document
>> that), but otherwise I think we should hold metadata to the same
>> standard as the rest of the IR.
>> 
> 
> The idea with metadata is that it can be removed and everything still
> works. I'm definitely not ready to lock down the debug metadata format
> and I really don't think we should for any of the other uses since
> stripping already works. (Note, I don't consider function attributes
> etc as metadata)
We may need to rethink this. If metadata is used only as optimization / codegen
hints, then yes I agree they can be dropped. But I suspect there is a need for
metadata that’s *required* for correctness. As LLVM continues to gain clients
beyond “just” compilers, we will need to be sensitive to their needs. I
anticipate use of LLVM bitcode files as persistent object format.

Evan
> 
> -eric
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

> On Jul 9, 2014, at 3:09 PM, Evan Cheng <evan.cheng at apple.com>
wrote:
> 
>> 
>> On Jun 17, 2014, at 1:24 PM, Rafael Espíndola <rafael.espindola at
gmail.com> wrote:
>> 
>> On 17 June 2014 16:07, Anton Korobeynikov <anton at
korobeynikov.info> wrote:
>>> Hi Rafael,
>>> 
>>>> Do others agree that this is the case or at least that this
would be a
>>>> reasonable balance?
>>> IMO it's easier to be compatible on .ll level, no?
>> 
>> That is not my experience with the bitcode format. The way the API is
>> structured makes it really easy to support backwards compatibility.
>> 
>> It also seems a lot more valuable from an user perspective to support
>> reading old .bc files. It means they can keep a library with IR for an
>> entire major LLVM release for example.
>> 
>>> In case of binary
>>> IR it's really easy to make incompatible changes. After all
there are
>>> no tests on IR binary compatibility, however the whole regression
>>> testsuite can be viewed as a big test for .ll compatibility.
>> 
>> We do have tests that are done by checking in old versions of bitcode
>> files. We didn't use to be good about it, but I think we are now
>> fairly systematic about it any time we change the format.
>> 
>>> There are two more points here:
>>> 
>>> 1. Actually we had much stronger policies wrt the bitcode
>>> compatibility in minor releases. Something like x.y should be able
to
>>> read stuff from x.y-1, but x.y+2 is allowed not to read stuff
there,
>>> so the proper path is transition x.y-1 => x.y => x.y+2. Am I
right?
>> 
>> That doesn't match what we have in trunk right now.  For example,
we
>> changed how inline asm is stored in r163185 (Sep 5 2012), but we still
>> support reading the old one. This is one of the cases where we have a
>> FIXME about 4.0.
> 
> My understanding is newer version of LLVM should be able read older version
with the same major release. And the .0 of the new major release must be able to
read the bitcode of the previous major release. I think this is the right
policy. We haven’t done a good job enforcing the policy, but we should.
That was also my understanding of our policy.

You are right that we need to do a better job of testing this. We have some
ad-hoc tests for particular bitcode auto-upgrades but nothing systematic. These
tests just read binary bitcode files, run them through llvm-dis, and check for
the expected output. An initial step to improve the situation would be to write
similar tests that attempt to exercise every IR feature, not just the things
that we already know need to be upgraded from old bitcode. We would need to
adopt the convention of extending these tests whenever something is added to the
IR. Whenever we make a release, we would make a copy this set of tests
corresponding to that release and we would continue to run those tests on trunk,
e.g., reading the bitcode from the 3.5 release. The expected output for these
tests might need to be updated occasionally. I would want to try to pack these
systematic tests into as few files as possible, ideally only one, so that it is
easier to make copies as part of the release process. I don’t know if that is
feasible, especially for checking target-specific intrinsics, but it would be a
nice goal.

Any feedback on that proposal?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140916/e4c3cc57/attachment.html>