[PATCH] D13088: [InlineCost] Adjust inlining cost for implicit null checks

Chen Li via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 25 10:37:08 PDT 2015


chenli added a comment.

In http://reviews.llvm.org/D13088#252355, @reames wrote:

> A a meta level, I feel this this patch is approaching the icmp part the wrong way.  Rather than trying to determine if a particular comparison is a implicit null check, we should check to see if a particular *branch* is likely implicit and if so, discount the cost of both the branch and the icmp.  I'm also not sure it's far to entirely discount the cost of both.  We can't from the IR guarantee that the check will actually be made implicit.


I dont feel like the icmp part should be handled in visitBranch. My reasoning is that the icmp might have other user as well. You should not treat it as free if only one of the users is implicit null check. You need to know all its users to decide whether it can be free. That's why I put the check in visitCmp instead of visitBranch.

I agree that it might not be proper to entirely discount the cost as there're chances the lowering part cannot make it implicit. I'd very like to hear more opinions on this.

> Also, can you add a bit of context to your description of the change?  I know what an implicit null check is, but not everyone will.


Thanks for mentioning. I will update the description.

> Chandler, since you're sure to have strong opinions, what do you feel is the right approach for framing this?



http://reviews.llvm.org/D13088





More information about the llvm-commits mailing list