[PATCH] D59922: [Attributor] Deduce "no-capture" argument attribute

[Hideto Ueno via Phabricator via cfe-commits]

uenoku accepted this revision.
uenoku added a comment.
This revision is now accepted and ready to land.

I think the logic is sound. It looks good to me.



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2618
+    else
+      Attrs.emplace_back(Attribute::get(Ctx, "no-capture-maybe-returned"));
+  }
----------------
Maybe we need an exact definition for `no-capture-maybe-returned` in LangRef.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2674
+  /// "no-capture-maybe-returned", the user is added to the \p PotentialCopies
+  /// set. All values in \p PotentialCopies are later tracked aswell. For every
+  /// explored use we decrement \p RemainingUsesToExplore. Once it reaches 0,
----------------
aswell


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2761
+  /// Register \p CS as potential copy of the value we are checking.
+  void addPotentialCopyIfNecessary(CallSite CS) {
+    PotentialCopies.push_back(CS.getInstruction());
----------------
Why `IfNecessary`? It seems there is no check.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2819
+
+  AAAlign::StateType T;
+  // TODO: Once we have memory behavior attributes we should use them here
----------------
Align?


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2864
+    // TODO: Once we have call site specific value information we can provide
+    //       call site specific liveness liveness information and then it makes
+    //       sense to specialize attributes for call sites arguments instead of
----------------
liveness liveness


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59922