[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