[PATCH] D14309: Use updated threshold for indirect call bonus

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 8 09:04:02 PST 2015


On Tue, Dec 8, 2015 at 1:54 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> chandlerc added a comment.
>
> While the source code change here makes sense, there is no test added which is not acceptable, and there was no explicit LGTM.
>
> David, just marking a patch as accepted in Phabricator doesn't send an email. Please actually write an LGTM in the comment box and send that so others are aware.
>

Good to know that.


> Also, David, I would suggest waiting for others to give a final approval for changes to the inline cost mechanisms,

This is a relatively simple change that was submitted more than a month ago.

>as this isn't really an area of LLVM that you've done extensive work on (yet).

While I agree with the statement -- I don't see how a good inliner and
analysis should be any different across different compilers.

We do need more reviewers in LLVM Inliner IMO :)

> Also, it isn't appropriate to LGTM this without a test that ensures we don't regress.

Acknowledged.

thanks,

David


>
> Easwaran, please add a test.
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D14309
>
>
>


More information about the llvm-commits mailing list