Jiong Wang
2013-Mar-23 15:32 UTC
[LLVMdev] About commit TILE-Gx backend to community repository and default disabled
äº 2013/3/23 23:14, Tobias Grosser åé:> On 03/23/2013 10:51 AM, Jiong Wang wrote: >> on 2013/3/23 1:52, Chris Lattner wrote: >>> On Mar 19, 2013, at 8:58 PM, Jiong Wang <jiwang at tilera.com> wrote: >>> >>>> Hi Chris, >>>> >>>> could you please comment on committing TILE-Gx backend into community? >>> Hi Jiong, >>> >>> I don't have any special advice here. It sounds like the general >>> functionality level is high enough. Taking it into mainline sounds >>> great, so long as it is reviewed by someone. >> >> thanks. >> >> I will commit after rebasing & re-testing the code on latest llvm >> mainline. And I will follow http://llvm.org/docs/HowToAddABuilder.html >> to setup a TILE-Gx buildbot. > > Hi Jiong, > > did you receive a full review by an LLVM committer who approved the > full patch set (not just individual ones) with some kind of "Looks > good to me"? This is the requirement Chris put for inclusion and only > after this has happened (and there are no major concerns found during > review) you should commit the back end. > > I am myself very supportive on getting this in, but I have the feeling > this did not yet happen. So I propose to not commit this backend > prematurely. (In case I missed something and this review already > happened, would you mind pointing me to the email and committer who > reviewed the backend)Hi Tobias, thanks for your reply, it's happy to receive more feedbacks. below are preivous community feedbacks, ==my first post to llvm-commit mailing list: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130304/167737.html my second post to llvm-commit mailing list which split the big patch into 17 small parts: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130311/167891.html Anton Korobeynikov, Jakob Stoklund Olesen, Joerg Sonnenberger given some feedback on coding style etc, all fixed. do you think it's OK ? please feel free to point out if there is anything missing. thank. --- Regards, Jiong Tilera Corporation.> > Thanks, > Tobias >
Tobias Grosser
2013-Mar-23 15:59 UTC
[LLVMdev] About commit TILE-Gx backend to community repository and default disabled
> > Hi Jiong, >> >> did you receive a full review by an LLVM committer who approved the >> full patch set (not just individual ones) with some kind of "Looks >> good to me"? This is the requirement Chris put for inclusion and only >> after this has happened (and there are no major concerns found during >> review) you should commit the back end. >> >> I am myself very supportive on getting this in, but I have the feeling >> this did not yet happen. So I propose to not commit this backend >> prematurely. (In case I missed something and this review already >> happened, would you mind pointing me to the email and committer who >> reviewed the backend) > > Hi Tobias, > > thanks for your reply, it's happy to receive more feedbacks. > > below are preivous community feedbacks, > > ==> my first post to llvm-commit mailing list: > http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130304/167737.html > > > my second post to llvm-commit mailing list which split the big patch > into 17 small parts: > > http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130311/167891.html > > Anton Korobeynikov, Jakob Stoklund Olesen, Joerg Sonnenberger given some > feedback on coding style etc, all fixed. > > do you think it's OK ? please feel free to point out if there is > anything missing.Yes, I have seen this and it is a very good start. Some of our core contributors are giving you feedback to this back-end and you are addressing their remarks. This is great. With Chris's reply you also got the general OK to upstream the patch, _AFTER_ the actual patches have been reviewed. I am unsure to which extend Anton, Jakob and Joerg have reviewed your back-end, but from the emails I have seen, I am not convinced they are finished. I personally believe for a patch set of this size, you should wait for an explicit email of one of the core contributors which states that the _full_ patch set has been reviewed and is good to commit or that an individual patch can be committed by itself. All the best, Tobias
Joerg Sonnenberger
2013-Mar-23 16:07 UTC
[LLVMdev] About commit TILE-Gx backend to community repository and default disabled
On Sat, Mar 23, 2013 at 04:59:49PM +0100, Tobias Grosser wrote:> With Chris's reply you also got the general OK to upstream the > patch, _AFTER_ the actual patches have been reviewed.One thing I was asking on IRC for is whether it makes sense for new backends to be committable in incremental steps. Especially for a complete backend like this, e.g. the asm parser would make a reasonable and useful part to get in and limit the amount of code to be reviewed at one point. Joerg
Reasonably Related Threads
- [LLVMdev] About commit TILE-Gx backend to community repository and default disabled
- [LLVMdev] About commit TILE-Gx backend to community repository and default disabled
- [LLVMdev] About commit TILE-Gx backend to community repository and default disabled
- [LLVMdev] About commit TILE-Gx backend to community repository and default disabled
- [LLVMdev] About commit TILE-Gx backend to community repository and default disabled