llvm dev - Mar 2013 - [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
>

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

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