[PATCH] D109170: [Attributor] Look through allocated heap memory

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 7 22:31:43 PDT 2021


jdoerfert added a comment.

In D109170#2988599 <https://reviews.llvm.org/D109170#2988599>, @kuter wrote:

> Also, do you think we can optimize nested stuff ? for example if a pointer was stored to memory.
> I think `checkForAllUses` can look through values that escape to memory,  right ?

It "works" but I discovered that we need one more piece that I will put in here or separately.
Basically, we need to connect the use that is stored with the copies that are loaded.



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:210
     return UndefValue::get(&Ty);
+  if (isNoAliasFn(&Obj, TLI)) {
+    if (isMallocLikeFn(&Obj, TLI) || isAlignedAllocLikeFn(&Obj, TLI))
----------------
kuter wrote:
> I don't think this check is needed.  `isNoAliasFn` seems to always return true if this is a allocation function.
> 
> Source code for `isNoAliasFn`
> ```
>  /// Tests if a value is a call or invoke to a function that returns a
>  /// NoAlias pointer (including malloc/calloc/realloc/strdup-like functions).
>  bool llvm::isNoAliasFn(const Value *V, const TargetLibraryInfo *TLI,
>                         bool LookThroughBitCast) {
>    // it's safe to consider realloc as noalias since accessing the original
>    // pointer is undefined behavior
>    return isAllocationFn(V, TLI, LookThroughBitCast) ||
>           hasNoAliasAttr(V, LookThroughBitCast);
>  }
> ```
> 
I don't understand why this would not be needed.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:2344
     AANoRecurseImpl::initialize(A);
+    // TODO: We should build a call graph ourselves to enable this in the module pass as well.
     if (const Function *F = getAnchorScope())
----------------
kuter wrote:
> Do you think we should use the `AAReachability` here ? 
> If the function can reach itself, then it is recursive. 
Yes, I'm looking into that already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109170



More information about the llvm-commits mailing list