[PATCH] D71617: [WIP][NFC][Attributor] noalias attribute deduction fixme

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Thanks for the update. I inlined some comments.



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:855
+                          const AbstractAttribute &QueryingAA,
+                          const Value &From);
+
----------------
Could we make the Predicate take the same values as for `checkForAllUses`?
Don't we need to specify (1) the uses of "what", and (2) reachable from "where". The `From` argument seems to mix these two concepts.

Two combine the two comments above: I guess we could actually make this a special of `checkForAllUses`. If `checkForAllUses` is given a `Instruction *ReachableFromI` argument that is not null, it means filter uses that are not reachable from `ReachableFromI`.




================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:1707
+    return getAssumed();
   }
 
----------------
I don't know why we want Values here and why you return assumed, the "worst" state, which is what we need to return until we get a proper `updateImpl` is `true`.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2257
     if (!NoCaptureAA.isAssumedNoCaptureMaybeReturned()) {
-      LLVM_DEBUG(
-          dbgs() << "[Attributor][AANoAliasCSArg] " << V
-                 << " cannot be noalias as it is potentially captured\n");
-      return indicatePessimisticFixpoint();
+      if (!A.checkReachableUses(UsePred, *this, getAssociatedValue())) {
+        LLVM_DEBUG(
----------------
What we need to check here is:

Are there uses of the pointer value that are potentially executed before the use at this call site *and* is the pointer potentially captured at any of those uses.

I think we could do this in two steps:

1) Iterate over all uses of the pointer and collect the uses that may reach the current call instruction. So I think the `A.checkForAllUses` might actually do as we need to check if the call we are looking at is reachable from the use.
2) For all those uses, ask AANoCapture if they are potentially capturing. The method doesn't exist yet but you can add it to the AANoCapture interface and for now return always `true`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71617