[PATCH] D21045: Use ProfileSummaryInfo in inline cost analysis

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 10 00:32:20 PDT 2016


On Thu, Jun 9, 2016 at 11:53 AM, Easwaran Raman <eraman at google.com> wrote:

> eraman added a comment.
>
> In http://reviews.llvm.org/D21045#450495, @silvas wrote:
>
> > Strictly speaking, I don't think this is NFC (e.g. it required changing
> the tests). But it seems straightforward enough.
>
>
> Sorry, I meant it in the sense of it doesn't change the functionality of
> inliner.
>
> David, IIRC patches need an explicit LGTM before submitting and accepting
> the patch in phabricator is not enough.
>

>From the community practices I've seen, accepting a patch in phabricator is
equivalent to LGTM so I think you are fine here.

(at some point in the past (or maybe still?) phabricator does not send
email when you mark a patch as accepted unless you also put a comment in
the comment box, so people often put "LGTM" there anyway)

Personally, I alway try to put "LGTM" in the comment box for the message
when I accept a revision as a bit of redundancy.

-- Sean Silva


>
>
> http://reviews.llvm.org/D21045
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160610/b61f1b2f/attachment.html>


More information about the llvm-commits mailing list