[PATCH] Teach InlineCost to account for a null check which can be folded away

Philip Reames listmail at philipreames.com
Mon Jun 15 17:34:34 PDT 2015


================
Comment at: lib/Analysis/IPA/InlineCost.cpp:525
@@ -516,1 +524,3 @@
 
+bool CallAnalyzer::isKnownNonNullInCallee(Value *V) {
+  // Does the *call site* have the NonNull attribute set on an argument?  We
----------------
chandlerc wrote:
> reames wrote:
> > chandlerc wrote:
> > > What do you think about making this a generic wrapper around CS.paramHasAttr rather than a not-null specific wrapper? We should be careful to *always* use this method to query for attributes, and I'm not sure we do today.
> > I'm not sure what the point of this would be.  Are you looking for something to limit access to the CallSite variable?  Or something else?
> > 
> > Just to make sure I read your question right, you were asking about a wrapper of the form:
> > bool CallAnalyzer::hasParamAttr(Value *V, Attributes::AttributeKind Attr)
> > 
> > Right?
> > 
> > (I'm not *opposed* to this; I just don't see the point of the extra abstraction.)
> The reason for suggesting this is that it makes it obvious how the next person should query some *other* attribute off of the call site in the CallAnalyzer. It also (IMO) makes it less likely that they'll do so incorrectly.
> 
> Essentially, I'm trying to separate the non-null property testing from the callsite attribute querying.
> 
> I would probably name it 'isArgumentWithAttribute(Value *V, Attributes::AttributeKind Attr)' or something along those lines.
I'm fine with this proposal and will upload a patch tomorrow with it done.  

p.s. Given I like the removal of copied code the isKnownNonNull extraction resulted in, I plan to keep both.

http://reviews.llvm.org/D9129

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list