[PATCH] D73428: [Attributor] Improve `noalias` deduction based on memory information

[Johannes Doerfert via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2772-2774
+    // If the definition is not `noalias` we need to check globals in addition
+    // to arguments. If the definition is `noalias` we cannot alias globals to
+    // begin with.
----------------
baziotis wrote:
> Why do we know that?
If we have already `noalias` (or an alloca) we know we do not alias any access in the scope not derived from this pointer. Since globals cannot be derived from a `noalias` pointer (in a way that would be problematic here) or in any way from an alloca, we know the global does not alias the pointer.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2792
+  bool
+  isKnownNoAliasAtCalleeDueToAccesses(Attributor &A,
+                                      const AAMemoryBehavior &MemBehaviorAA,
----------------
baziotis wrote:
> Maybe doc here (?) It looks similar to the above but some things I think would be good to be mentioned like why do we test for globals here.
Will write a doc :)


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2805
+      LLVM_DEBUG(
+          dbgs() << "[AANoAlias] Check access by " << I << " to " << Ptr << " ["
+                 << AAMemoryLocation::getMemoryLocationsAsStr(MLK) << "]\n");
----------------
baziotis wrote:
> Maybe you want `*Ptr` here. Also maybe conditioned on `Ptr` (like above).
Will do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73428