[PATCH] D16381: Infrastructure to allow use of PGO in inliner

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 7 09:47:07 PST 2016


On Mon, Mar 7, 2016 at 1:40 AM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> chandlerc added a comment.
>
> In http://reviews.llvm.org/D16381#367348, @davidxl wrote:
>
> > LGTM.
> >
> > This looks really great, so let's move on with this long waited missing
> feature.
>
>
> David, this is not an area of LLVM you have done substantial work on, and
> this is a very significant feature.
>

The patch has been reviewed and tested thoroughly. The approach used by the
patch has been discussed many times in the past since last October in
various contexts in which you were involved.


>
> I'm sorry that I have not had time to review this yet, but the correct
> response is not for you to make a patch as LGTM.



Again we have done extensive review on this patch. According to your
standard, it looks like only you can approve patches in the inliner?  IMO
this is not healthy to the project and more people need to get involved. As
it stands today, it seems to be very hard to everybody in the community to
make any changes in the inliner due to this.



> Please revert this and let's actually get it reviewed before it goes into
> the project.
>
>
Actually get reviewed?  What do you mean? Have you really followed the
thread?   If you want to be constructive, please do your part of the
review, but not request a revert like this.

David







>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D16381
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160307/13683c0a/attachment.html>


More information about the llvm-commits mailing list