[MemoryBuiltins] Remove isNoAliasFn() in favor of isNoAliasCall()

We currently have two similar implementations of this concept:
isNoAliasCall() only checks for the noalias return attribute.
isNoAliasFn() also checks for allocation functions.

We should switch to only checking the attribute. SLC is responsible
for inferring the noalias return attribute for non-new allocation
functions (with a missing case fixed in
https://github.com/llvm/llvm-project/commit/348bc76e3548c52dbcd442590ca0a7f5b09b7534).
For new, clang is responsible for setting the attribute,
if -fno-assume-sane-operator-new is not passed.

Differential Revision: https://reviews.llvm.org/D116800

7 files changed, 6 insertions, 32 deletions

diff --git a/llvm/include/llvm/Analysis/MemoryBuiltins.h b/llvm/include/llvm/Analysis/MemoryBuiltins.h
index 28104c5ed420..95881bd2480e 100644
--- a/llvm/include/llvm/Analysis/MemoryBuiltins.h
+++ b/llvm/include/llvm/Analysis/MemoryBuiltins.h
@@ -56,10 +56,6 @@ bool isAllocationFn(const Value *V, const TargetLibraryInfo *TLI);
 bool isAllocationFn(const Value *V,
                     function_ref<const TargetLibraryInfo &(Function &)> GetTLI);
 
-/// 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 isNoAliasFn(const Value *V, const TargetLibraryInfo *TLI);
-
 /// Tests if a value is a call or invoke to a library function that
 /// allocates uninitialized memory (such as malloc).
 bool isMallocLikeFn(const Value *V, const TargetLibraryInfo *TLI);
diff --git a/llvm/lib/Analysis/MemoryBuiltins.cpp b/llvm/lib/Analysis/MemoryBuiltins.cpp
index fe21ab45d85d..86e711884e2e 100644
--- a/llvm/lib/Analysis/MemoryBuiltins.cpp
+++ b/llvm/lib/Analysis/MemoryBuiltins.cpp
@@ -222,11 +222,6 @@ static Optional<AllocFnsTy> getAllocationSize(const Value *V,
   return Result;
 }
 
-static bool hasNoAliasAttr(const Value *V) {
-  const auto *CB = dyn_cast<CallBase>(V);
-  return CB && CB->hasRetAttr(Attribute::NoAlias);
-}
-
 /// Tests if a value is a call or invoke to a library function that
 /// allocates or reallocates memory (either malloc, calloc, realloc, or strdup
 /// like).
@@ -238,15 +233,6 @@ bool llvm::isAllocationFn(
   return getAllocationData(V, AnyAlloc, GetTLI).hasValue();
 }
 
-/// 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) {
-  // it's safe to consider realloc as noalias since accessing the original
-  // pointer is undefined behavior
-  return isAllocationFn(V, TLI) ||
-         hasNoAliasAttr(V);
-}
-
 /// Tests if a value is a call or invoke to a library function that
 /// allocates uninitialized memory (such as malloc).
 bool llvm::isMallocLikeFn(const Value *V, const TargetLibraryInfo *TLI) {
diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index da6bb4c49cba..36df462c7a66 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -594,7 +594,7 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
     // turn into undef.  Note that we can bypass the allocation itself when
     // looking for a clobber in many cases; that's an alias property and is
     // handled by BasicAA.
-    if (isa<AllocaInst>(Inst) || isNoAliasFn(Inst, &TLI)) {
+    if (isa<AllocaInst>(Inst) || isNoAliasCall(Inst)) {
       const Value *AccessPtr = getUnderlyingObject(MemLoc.Ptr);
       if (AccessPtr == Inst || BatchAA.isMustAlias(Inst, AccessPtr))
         return MemDepResult::getDef(Inst);
diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index a71c70b3fbca..12b8a0ef9d00 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -209,7 +209,6 @@ Constant *AA::getInitialValueForObj(Value &Obj, Type &Ty,
     return UndefValue::get(&Ty);
   if (isAllocationFn(&Obj, TLI))
     return getInitialValueOfAllocation(&cast<CallBase>(Obj), TLI, &Ty);
-
   auto *GV = dyn_cast<GlobalVariable>(&Obj);
   if (!GV || !GV->hasLocalLinkage())
     return nullptr;
@@ -305,8 +304,6 @@ bool AA::getPotentialCopiesOfStoredValue(
   SmallVector<const AAPointerInfo *> PIs;
   SmallVector<Value *> NewCopies;
 
-  const auto *TLI =
-      A.getInfoCache().getTargetLibraryInfoForFunction(*SI.getFunction());
   for (Value *Obj : Objects) {
     LLVM_DEBUG(dbgs() << "Visit underlying object " << *Obj << "\n");
     if (isa<UndefValue>(Obj))
@@ -324,7 +321,7 @@ bool AA::getPotentialCopiesOfStoredValue(
       return false;
     }
     if (!isa<AllocaInst>(Obj) && !isa<GlobalVariable>(Obj) &&
-        !isNoAliasFn(Obj, TLI)) {
+        !isNoAliasCall(Obj)) {
       LLVM_DEBUG(dbgs() << "Underlying object is not supported yet: " << *Obj
                         << "\n";);
       return false;
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 07c16c7d6b70..a78517425fd0 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -5274,9 +5274,6 @@ struct AAValueSimplifyImpl : AAValueSimplify {
           continue;
         return false;
       }
-      if (!isa<AllocaInst>(Obj) && !isa<GlobalVariable>(Obj) &&
-          !isNoAliasFn(Obj, TLI))
-        return false;
       Constant *InitialVal = AA::getInitialValueForObj(*Obj, *L.getType(), TLI);
       if (!InitialVal || !Union(*InitialVal))
         return false;
diff --git a/llvm/test/Transforms/GVN/malloc-load-removal.ll b/llvm/test/Transforms/GVN/malloc-load-removal.ll
index bbb6f5ca7851..57fb89225efd 100644
--- a/llvm/test/Transforms/GVN/malloc-load-removal.ll
+++ b/llvm/test/Transforms/GVN/malloc-load-removal.ll
@@ -5,7 +5,7 @@
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.8.0"
 
-declare i8* @malloc(i64) nounwind
+declare noalias i8* @malloc(i64) nounwind
 
 define noalias i8* @test1() nounwind uwtable ssp {
 entry:
@@ -30,7 +30,7 @@ if.end:                                           ; preds = %if.then, %entry
 ; CHECK_NO_LIBCALLS: icmp
 }
 
-declare i8* @_Znwm(i64) nounwind
+declare noalias i8* @_Znwm(i64) nounwind
 
 define noalias i8* @test2() nounwind uwtable ssp {
 entry:
@@ -55,7 +55,7 @@ if.end:                                           ; preds = %if.then, %entry
 ; CHECK_NO_LIBCALLS: icmp
 }
 
-declare i8* @aligned_alloc(i64, i64) nounwind
+declare noalias i8* @aligned_alloc(i64, i64) nounwind
 
 define noalias i8* @test3() nounwind uwtable ssp {
 entry:
diff --git a/llvm/unittests/Analysis/MemoryBuiltinsTest.cpp b/llvm/unittests/Analysis/MemoryBuiltinsTest.cpp
index 575b7f320473..05f1a586f504 100644
--- a/llvm/unittests/Analysis/MemoryBuiltinsTest.cpp
+++ b/llvm/unittests/Analysis/MemoryBuiltinsTest.cpp
@@ -36,14 +36,12 @@ TEST(AllocSize, AllocationBuiltinsTest) {
       CallInst::Create(AllocSizeFn, {ConstantInt::get(ArgTy, 100)}));
 
   const TargetLibraryInfo *TLI = nullptr;
-  EXPECT_FALSE(isNoAliasFn(Caller.get(), TLI));
   EXPECT_FALSE(isMallocLikeFn(Caller.get(), TLI));
   EXPECT_FALSE(isCallocLikeFn(Caller.get(), TLI));
   EXPECT_FALSE(isAllocLikeFn(Caller.get(), TLI));
 
   // FIXME: We might be able to treat allocsize functions as general allocation
-  // functions. For the moment, being conservative seems better (and we'd have
-  // to plumb stuff around `isNoAliasFn`).
+  // functions.
   EXPECT_FALSE(isAllocationFn(Caller.get(), TLI));
 }
 }