[PATCH] D67886: NoFree argument attribute.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1513
+  }
+};
+
----------------
sstefan1 wrote:
> jdoerfert wrote:
> > I would put the logic that follows uses into the floating version and make everything that is not function scope (function & call site) derive from floating. (We need to have a generic use visitor but that is a different story)
> I can do that. Just want to make sure I understand what you are saying. I should keep the current function & call site implementations, right?
> 
> I also thought about generic use visitor, I'll think about it and maybe write a patch this week.
Keep function & call site as is, yes.

If you want to work on a use visitor that is fine with me. Having edge-wise liveness information is also up there on the list of things I'd like to have.


================
Comment at: llvm/test/Transforms/FunctionAttrs/nofree-attributor.ll:270
 attributes #1 = { nounwind }
 attributes #2 = { nobuiltin nounwind }
----------------
sstefan1 wrote:
> jdoerfert wrote:
> > We need tests where one argument is freed and one is not. Also where either one of two is freed but we don't know which (e.g., due to a phi). And we need uncertainty tests, e.g. the argument escapes and the function (wrt. to the argument) is not nofree.
> I will add more tests soon. Actually a lot of tests will be affected, so maybe we are already covering some of the things you are mentioning.
OK, we could add them either way to make it explicit ;)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67886/new/

https://reviews.llvm.org/D67886