On Mon, Jul 25, 2016 at 5:47 PM Mehdi Amini via llvm-dev < llvm-dev at lists.llvm.org> wrote:> > > On Jul 25, 2016, at 4:18 PM, Renato Golin via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > > 6. The target's code must have been adapted to the developers policy as > well as > > the coding standards. This can take a while and it should be fine to > > accept external code into LLVM as experimental, but not officially. > > FWIW I’m not fond of not having this as the experimental part in the first > place. > It is harder to have things moving after being upstreamed. > I’d expect the upstreaming of a backend (into experimental state) to be > piece by piece with proper code review according to the current project > standard. >I'm not sure it makes sense to insist on backends being incrementally developed in the open. While I rather prefer that (WebAssembly has been fun to watch here), it doesn't seem realistic. Every backend I can think of other than WebAssembly and the original AArch64 port has come into existence out of tree, and only after proving useful to some folks decided to move into the tree. I'd personally advocate for keeping that door open. All that said,I completely agree regarding coding conventions and other basic stuff. I don't think we need to wait for the official point in time to insist on all of the fundamental coding standards and such being followed.> > > > > A soft(-er?) requirement: > > > > 7. The test coverage is broad and well written (small tests, > documented). Where > > applicable, both the ``check-all`` and ``test-suite`` must pass in at > least > > one configuration (publicly demonstrated, ex. via buildbots). > > Same as before: this is a no-brainer should be applicable with any patch, > even in experimental state: small and documented lit tests. >Yep, this seems like it should always be true regardless of the experimental nature. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160726/8bdcd4a1/attachment.html>
> On Jul 25, 2016, at 5:54 PM, Chandler Carruth <chandlerc at google.com> wrote: > > On Mon, Jul 25, 2016 at 5:47 PM Mehdi Amini via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > > > On Jul 25, 2016, at 4:18 PM, Renato Golin via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > > 6. The target's code must have been adapted to the developers policy as well as > > the coding standards. This can take a while and it should be fine to > > accept external code into LLVM as experimental, but not officially. > > FWIW I’m not fond of not having this as the experimental part in the first place. > It is harder to have things moving after being upstreamed. > I’d expect the upstreaming of a backend (into experimental state) to be piece by piece with proper code review according to the current project standard. > > I'm not sure it makes sense to insist on backends being incrementally developed in the open. While I rather prefer that (WebAssembly has been fun to watch here), it doesn't seem realistic. > > Every backend I can think of other than WebAssembly and the original AArch64 port has come into existence out of tree, and only after proving useful to some folks decided to move into the tree. I'd personally advocate for keeping that door open.There is a nuance: the fact that it is not developed in tree does not prevent from splitting it in “some” pieces that can be independently reviewed, this is what we (should) do for any large chunk of code that we integrate. Now it is possible that considering the current way of implementing backend in LLVM, there is not much pieces that can be split out of a backend (I still think that’s unfortunate though). — Mehdi> > All that said,I completely agree regarding coding conventions and other basic stuff. I don't think we need to wait for the official point in time to insist on all of the fundamental coding standards and such being followed. > > > > > > A soft(-er?) requirement: > > > > 7. The test coverage is broad and well written (small tests, documented). Where > > applicable, both the ``check-all`` and ``test-suite`` must pass in at least > > one configuration (publicly demonstrated, ex. via buildbots). > > Same as before: this is a no-brainer should be applicable with any patch, even in experimental state: small and documented lit tests. > > Yep, this seems like it should always be true regardless of the experimental nature.-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160725/9e85185f/attachment.html>
On Mon, Jul 25, 2016 at 6:22 PM Mehdi Amini <mehdi.amini at apple.com> wrote:> On Jul 25, 2016, at 5:54 PM, Chandler Carruth <chandlerc at google.com> > wrote: > > On Mon, Jul 25, 2016 at 5:47 PM Mehdi Amini via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> >> > On Jul 25, 2016, at 4:18 PM, Renato Golin via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> > 6. The target's code must have been adapted to the developers policy as >> well as >> > the coding standards. This can take a while and it should be fine to >> > accept external code into LLVM as experimental, but not officially. >> >> FWIW I’m not fond of not having this as the experimental part in the >> first place. >> It is harder to have things moving after being upstreamed. >> I’d expect the upstreaming of a backend (into experimental state) to be >> piece by piece with proper code review according to the current project >> standard. >> > > I'm not sure it makes sense to insist on backends being incrementally > developed in the open. While I rather prefer that (WebAssembly has been fun > to watch here), it doesn't seem realistic. > > Every backend I can think of other than WebAssembly and the original > AArch64 port has come into existence out of tree, and only after proving > useful to some folks decided to move into the tree. I'd personally advocate > for keeping that door open. > > > There is a nuance: the fact that it is not developed in tree does not > prevent from splitting it in “some” pieces that can be independently > reviewed, this is what we (should) do for any large chunk of code that we > integrate. > Now it is possible that considering the current way of implementing > backend in LLVM, there is not much pieces that can be split out of a > backend (I still think that’s unfortunate though). >Sure, we should definitely split it in ways that make sense. I was just pointing out that this won't be the same thing as what "incremental development" would have naturally led to if the backend had been in tree on day-one, and while we actually insist on nearly replaying incremental development for some things, it doesn't seem likely to make sense for a backend.>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160726/8cc825f0/attachment.html>
> On Jul 25, 2016, at 5:54 PM, Chandler Carruth via llvm-dev <llvm-dev at lists.llvm.org> wrote: > >> On Mon, Jul 25, 2016 at 5:47 PM Mehdi Amini via llvm-dev <llvm-dev at lists.llvm.org> wrote: >> >> > On Jul 25, 2016, at 4:18 PM, Renato Golin via llvm-dev <llvm-dev at lists.llvm.org> wrote: >> > 6. The target's code must have been adapted to the developers policy as well as >> > the coding standards. This can take a while and it should be fine to >> > accept external code into LLVM as experimental, but not officially. >> >> FWIW I’m not fond of not having this as the experimental part in the first place. >> It is harder to have things moving after being upstreamed. >> I’d expect the upstreaming of a backend (into experimental state) to be piece by piece with proper code review according to the current project standard. > > I'm not sure it makes sense to insist on backends being incrementally developed in the open. While I rather prefer that (WebAssembly has been fun to watch here), it doesn't seem realistic. > > Every backend I can think of other than WebAssembly and the original AArch64 port has come into existence out of tree, and only after proving useful to some folks decided to move into the tree. I'd personally advocate for keeping that door open.I strongly agree with Chandler here. I *also* feel that gating the move from experimental to non-experimental would be a reasonable place to draw a line in the sand on this topic if desired. I am not arguing for actually drawing that strong a line mind you, just saying that would be a better place if we had to have one.> > All that said,I completely agree regarding coding conventions and other basic stuff. I don't think we need to wait for the official point in time to insist on all of the fundamental coding standards and such being followed. > >> >> > >> > A soft(-er?) requirement: >> > >> > 7. The test coverage is broad and well written (small tests, documented). Where >> > applicable, both the ``check-all`` and ``test-suite`` must pass in at least >> > one configuration (publicly demonstrated, ex. via buildbots). >> >> Same as before: this is a no-brainer should be applicable with any patch, even in experimental state: small and documented lit tests. > > Yep, this seems like it should always be true regardless of the experimental nature. > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160725/96e5286a/attachment.html>
> On Jul 25, 2016, at 7:13 PM, Philip Reames <listmail at philipreames.com> wrote: > > > > > > On Jul 25, 2016, at 5:54 PM, Chandler Carruth via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > >> On Mon, Jul 25, 2016 at 5:47 PM Mehdi Amini via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: >> >> > On Jul 25, 2016, at 4:18 PM, Renato Golin via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: >> > 6. The target's code must have been adapted to the developers policy as well as >> > the coding standards. This can take a while and it should be fine to >> > accept external code into LLVM as experimental, but not officially. >> >> FWIW I’m not fond of not having this as the experimental part in the first place. >> It is harder to have things moving after being upstreamed. >> I’d expect the upstreaming of a backend (into experimental state) to be piece by piece with proper code review according to the current project standard. >> >> I'm not sure it makes sense to insist on backends being incrementally developed in the open. While I rather prefer that (WebAssembly has been fun to watch here), it doesn't seem realistic. >> >> Every backend I can think of other than WebAssembly and the original AArch64 port has come into existence out of tree, and only after proving useful to some folks decided to move into the tree. I'd personally advocate for keeping that door open. > > I strongly agree with Chandler here. > > I *also* feel that gating the move from experimental to non-experimental would be a reasonable place to draw a line in the sand on this topic if desired. I am not arguing for actually drawing that strong a line mind you, just saying that would be a better place if we had to have one.I don’t make sense of this, so I guess we are not talking about the same thing. Moving from experimental to non-experimental is a one-line patch while all the pieces are glued together in-tree, and I don’t see any practical way to have some modular review of a backend *after* this happened. If you accept some spaghetti code as experimental, I don’t believe it is ever gonna be totally untangled by iterating on it in-tree. — Mehdi> > >> >> All that said,I completely agree regarding coding conventions and other basic stuff. I don't think we need to wait for the official point in time to insist on all of the fundamental coding standards and such being followed. >> >> >> > >> > A soft(-er?) requirement: >> > >> > 7. The test coverage is broad and well written (small tests, documented). Where >> > applicable, both the ``check-all`` and ``test-suite`` must pass in at least >> > one configuration (publicly demonstrated, ex. via buildbots). >> >> Same as before: this is a no-brainer should be applicable with any patch, even in experimental state: small and documented lit tests. >> >> Yep, this seems like it should always be true regardless of the experimental nature. >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160725/2d5e42f7/attachment.html>
On 26 July 2016 at 01:54, Chandler Carruth <chandlerc at google.com> wrote:> Every backend I can think of other than WebAssembly and the original AArch64 > port has come into existence out of tree, and only after proving useful to > some folks decided to move into the tree. I'd personally advocate for > keeping that door open.Indeed. Minimum requirements, not expected target.> All that said,I completely agree regarding coding conventions and other > basic stuff. I don't think we need to wait for the official point in time to > insist on all of the fundamental coding standards and such being followed.Indeed. Expected target, not minimum requirements. :) Bu having them separated, we're telling people that the latter is what they should strive for, but we're happy to help them along the way.>> > 7. The test coverage is broad and well written (small tests, >> > documented). > > Yep, this seems like it should always be true regardless of the experimental > nature.I think this is the same as above. New targets are generally introduced by people that don't know LLVM too well, or haven't written a full back-end before. Expecting *full* coverage and perfect tests from day one is as unreasonable as expecting them to fit all the required code style and other bits. But in order for it to be relevant upstream (ie. no maintenance burden), the tests need to be in good quality. After all, a lot of unrelated buildbots will be running *their* tests. It's the same thing: start small, grow up, become official. All in all, we're here to help them grow. That's the message I'm trying to pass along. cheers, --renato