[PATCH] D67886: NoFree argument attribute.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

quick feedback



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1546
+  AANoFreeCallSiteReturned(const IRPosition &IRP) : AANoFreeFloating(IRP) {}
+
+  /// See AbstractAttribute::trackStatistics()
----------------
jdoerfert wrote:
> overwrite `manifest` here to ensure we do not add "no-free" to the return value even if we derive it. We can actually derive it as we do not need to restrict it to arguments in the `AANoFreeFloating::update`.
This is still open, see my "why arguments" comment above.


Btw. later we can actually add "nofree" to return values (and call site returns) as it can help to keep dereferenceable. But that is for a follow up patch after we get `dereferenceable_globally`


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1433
+    for (Use &U : Arg->uses())
+      Worklist.push_back(&U);
+
----------------
Why do we need an argument and not just the associated value?


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1478
+      }
+
+      // Unknown user.
----------------
Allow PHI nodes and selects as well. Same as bitcast above.


================
Comment at: llvm/test/Transforms/FunctionAttrs/nofree-attributor.ll:267
+
+define void @test14(i8* nocapture %0, i8* nocapture %1) {
+    tail call void @free(i8* %0) #1
----------------
ATTRIBUTOR: check line missing

Also replace the CHECK with ATTRIBUTOR


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67886