<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, Mar 23, 2013 at 12:34 PM, Jiong Wang <span dir="ltr"><<a href="mailto:jiwang@tilera.com" target="_blank">jiwang@tilera.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">于 2013/3/24 0:07, Joerg Sonnenberger 写道:<div class="im"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Sat, Mar 23, 2013 at 04:59:49PM +0100, Tobias Grosser wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
With Chris's reply you also got the general OK to upstream the<br>
patch, _AFTER_ the actual patches have been reviewed.<br>
</blockquote>
One thing I was asking on IRC for is whether it makes sense for new<br>
backends to be committable in incremental steps. Especially for a<br>
complete backend like this, e.g. the asm parser would make a reasonable<br>
and useful part to get in and limit the amount of code to be reviewed at<br>
</blockquote>
<br></div>
Hi Tobias & Joerg,<br>
<br>
I understand your concern.<br>
<br>
yes, Asm Parser is a module which could be committed individually, but I think the code<br>
for AsmParser is relatively small and highly target dependent, a full regression test which<br>
cover all instructions could largly gurantee the correctness.<br>
<br>
I know contributors have their own works. For TILE-Gx backend, it's good if any contributor could have time to finish a full set review, I have been wating for several weeks.<br>
<br>
I personly think it's not premature to commit TILE-Gx backend into mainline and default disable. Because:<br>
<br>
1. TILE-Gx finished the regression test & test-suite test.<br>
2. TILE-Gx buildbot will be setup, so it will be convinent for both community & Tilera company to monitor the status.<br>
<br>
So should I keep waiting the full set review? or it's OK to commit and default disabled?<div class="im"><br></div></blockquote><div><br></div><div style>The AArch64 backend was committed fairly quickly after being proposed for inclusion, and they did not split up the patch and most of their backend was committed in one huge commit.</div>
<div style><br></div><div style>Although IIRC Bill Wendling (CC'd) explicitly asked you to split the backend into separate patches, it might be best to put it back together again for the final review. AFAIK Phabricator does not have a good way to keep all the patches somehow together, and otherwise they just get lost in people's mail (and it would be annoying to ping 10+ patches). Having a single patch avoids this problem and makes it a lot easier to keep track of the backend. Phabricator has good support for commenting on specific parts of the patch, so as long as the backend components are named in a recognizable way (I think they are), experienced reviewers should not have significant difficulty navigating (and they can always apply the patch locally if necessary, which is easier with one huge patch).</div>
<div style><br></div><div style>-- Sean Silva</div></div><br></div></div>