[PATCH] D67886: NoFree argument attribute.

[Stefan Stipanovic via Phabricator via llvm-commits]

sstefan1 marked 3 inline comments as done.
sstefan1 added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1419
+    Argument *Arg = getAssociatedArgument();
+    Function *F = Arg->getParent();
+
----------------
jdoerfert wrote:
> Look at the function attribute first, as long as it is set we do not have to prove anything but can just keep the optimistic state.
Ok, I agree. This is how I should have done it.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1513
+  }
+};
+
----------------
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.


================
Comment at: llvm/test/Transforms/FunctionAttrs/nofree-attributor.ll:270
 attributes #1 = { nounwind }
 attributes #2 = { nobuiltin nounwind }
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67886