[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