Renato Golin
2014-Sep-29 08:27 UTC
[LLVMdev] [cfe-dev] Proposal to add Bitcode version field to bitcode file wrapper
On 28 September 2014 23:10, Robinson, Paul <Paul_Robinson at playstation.sony.com> wrote:> | Bitcode backward compatibility, at least for the current major version, is > supposed to make this unnecessary.It didn't use to work that way, and I'm not sure we want it at all.> I think the "at least for the current major version" part is one thing that > concerns us. LLVM 4.0 will promise to read LLVM 3.4 bitcode, but LLVM 4.1 > will not, according to my understanding of the current promise.I've never heard such promises, but even if you're right (that there is a promise), we cannot enforce it, since we have no tests to make sure we do. Right now, the only guarantee I know exists (and it's a new one, from 3.4 onwards) is that minor releases won't break ABI or API compatibility, which includes IR logic. So 3.4.2 is guaranteed to parse 3.4 IR but not 3.3 or 3.5.x.> Smoothly identifying that point and being able to provide an intelligent diagnostic > seems like goodness. Hard to distinguish "old" bitcode from "broken" bitcode > without recording version info of some kind, and the sooner we start > recording the version number the more completely we're able to diagnose the > situation properly when the time comes.There are two problems with this: 1. Due to the nature of our development strategy, IR compatibility can be broken between two releases, which means any two commits within the same revision can fail to parse (or parse incorrectly) IR from each other. Do we care about between-release compatibility? Some people get specific commits, rather than releases, for timing reasons, for their products, and in doing so, you could get a commit that is actually IR incompatible with the next major release. If you care about compatibility, you should increment the IR version every time something radical changes, which can be multiple times between the same two releases, or spawn across multiple releases. IR versioning should be completely independent of major / minor release cycles. The hard part is to truly detect, and validate, IR compatibility changes. 2. IR incompatibility is different from metadata incompatibility. If the IR is incompatible (say we drop or add a new type, or we change how exceptions are propagated), the new parser will not understand the old and vice-versa. But if metadata changes, it can still be parsed, and as David said, if we can't understand it, we just drop it. If you want your parser to break the least, you'll have to have at least two version: IR and Debug. Other metadata versioning can be done individually (since they change at different rates). You may want to warn on stale metadata status (since it's not an error), but you should stop on stale IR. Finally, both problems end up in the same place: how do you validate this? We'd have to add a new class of tests, and for every new change in IR/metadata, we'd increase the version number and create a test that checks old parser+new syntax and old syntax+new parser and makes sure they fail/warn. You'd also need to have a table of major releases vs. IR versions, so that in the error/warning message you tell: please use LLVM M.N instead. cheers, --renato
Robinson, Paul
2014-Sep-29 14:16 UTC
[LLVMdev] [cfe-dev] Proposal to add Bitcode version field to bitcode file wrapper
> -----Original Message----- > From: Renato Golin [mailto:renato.golin at linaro.org] > Sent: Monday, September 29, 2014 1:27 AM > To: Robinson, Paul > Cc: Greg Bedwell; Sean Silva; Yung, Douglas; cfe-dev at cs.uiuc.edu; > llvmdev at cs.uiuc.edu > Subject: Re: [LLVMdev] [cfe-dev] Proposal to add Bitcode version field to > bitcode file wrapper > > On 28 September 2014 23:10, Robinson, Paul > <Paul_Robinson at playstation.sony.com> wrote: > > | Bitcode backward compatibility, at least for the current major > version, is > > supposed to make this unnecessary. > > It didn't use to work that way, and I'm not sure we want it at all. > > > > I think the "at least for the current major version" part is one thing > that > > concerns us. LLVM 4.0 will promise to read LLVM 3.4 bitcode, but LLVM > 4.1 > > will not, according to my understanding of the current promise. > > I've never heard such promises, but even if you're right (that there > is a promise), we cannot enforce it, since we have no tests to make > sure we do.That promise is what I understood from a discussion within the past month, e.g. http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-September/076815.html If I misunderstood, clarification on the clarification would be helpful. ;-) And recently tests have appeared that I thought were intended to validate this sort of thing, e.g. r218297. --paulr> > Right now, the only guarantee I know exists (and it's a new one, from > 3.4 onwards) is that minor releases won't break ABI or API > compatibility, which includes IR logic. So 3.4.2 is guaranteed to > parse 3.4 IR but not 3.3 or 3.5.x. > > > > Smoothly identifying that point and being able to provide an intelligent > diagnostic > > seems like goodness. Hard to distinguish "old" bitcode from "broken" > bitcode > > without recording version info of some kind, and the sooner we start > > recording the version number the more completely we're able to diagnose > the > > situation properly when the time comes. > > There are two problems with this: > > 1. Due to the nature of our development strategy, IR compatibility can > be broken between two releases, which means any two commits within the > same revision can fail to parse (or parse incorrectly) IR from each > other. Do we care about between-release compatibility? > > Some people get specific commits, rather than releases, for timing > reasons, for their products, and in doing so, you could get a commit > that is actually IR incompatible with the next major release. If you > care about compatibility, you should increment the IR version every > time something radical changes, which can be multiple times between > the same two releases, or spawn across multiple releases. > > IR versioning should be completely independent of major / minor > release cycles. The hard part is to truly detect, and validate, IR > compatibility changes. > > 2. IR incompatibility is different from metadata incompatibility. If > the IR is incompatible (say we drop or add a new type, or we change > how exceptions are propagated), the new parser will not understand the > old and vice-versa. But if metadata changes, it can still be parsed, > and as David said, if we can't understand it, we just drop it. > > If you want your parser to break the least, you'll have to have at > least two version: IR and Debug. Other metadata versioning can be done > individually (since they change at different rates). You may want to > warn on stale metadata status (since it's not an error), but you > should stop on stale IR. > > > Finally, both problems end up in the same place: how do you validate > this? We'd have to add a new class of tests, and for every new change > in IR/metadata, we'd increase the version number and create a test > that checks old parser+new syntax and old syntax+new parser and makes > sure they fail/warn. > > You'd also need to have a table of major releases vs. IR versions, so > that in the error/warning message you tell: please use LLVM M.N > instead. > > cheers, > --renato
Renato Golin
2014-Sep-29 14:29 UTC
[LLVMdev] [cfe-dev] Proposal to add Bitcode version field to bitcode file wrapper
On 29 September 2014 15:16, Robinson, Paul <Paul_Robinson at playstation.sony.com> wrote:> That promise is what I understood from a discussion within the past month, > e.g. http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-September/076815.html > If I misunderstood, clarification on the clarification would be helpful. ;-)I see, I missed that one. My concerns seem to be similar to Bob's, though in the past, when we discussed the same topic, there was one major hurdle to implement that: we'd have to know what features we removed / stopped supporting and warn on what version brackets supported that feature. This can only grow as the compiler ages. Enforcing backwards compatibility with only the major version created another hurdle: we'd only be able to deprecate bad/temporary features every few years, creating another bag of legacy. Warnings can be made, and deprecation of whole sets of features will happen at major version, which will stress the release validation and increase the influx of bugs on all major releases. Whenever I think of any of that, I remember Chris' words: "LLVM IR is a compiler IR. Nothing more, nothing less". I don't think we should try to standardise that too much. My tuppence. --renato