[PATCH] D49165: Add, and infer, a nofree function attribute

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:1228
+  StringRef FnName = Callee->getName();
+  if (TLI.getLibFunc(FnName, TLIFn))
+    return false;
----------------
hfinkel wrote:
> efriedma wrote:
> > This seems delicate; there are some C library functions which aren't "free", but can still free memory (like mmap).  Not sure we recognize any functions like that at the moment, but we could add them.  I'd prefer to add explicit logic to inferLibFuncAttributes.
> > 
> > If you are going to call getLibFunc, use the overload that takes a CallSite.
> > This seems delicate; there are some C library functions which aren't "free", but can still free memory (like mmap). Not sure we recognize any functions like that at the moment, but we could add them. I'd prefer to add explicit logic to inferLibFuncAttributes.
> 
> This also makes me uneasy. On the other hand, maintaining a separate list of functions which don't free memory, which is nearly all of them, also seems bad. My best suggestion is to add a strongly-worded note to include/llvm/Analysis/TargetLibraryInfo.def about what to do if you add such a function that frees memory. Would that be okay?
> 
I guess it's okay to default all known functions which aren't free-like/realloc-like to infer nofree, with an appropriate comment.

I'd still prefer to move the code to infer nofree on library calls to inferLibFuncAttributes.


Repository:
  rL LLVM

https://reviews.llvm.org/D49165