[BasicAA] Remove special casing of memset_pattern16 in favor of generic attribute inference

[reames]

Summary

BasicAA doesn't need to know about memset_pattern16. Since this rule was added, we've added attributes which are sufficient to encode all relevant aliasing facts. (Well, except for the ones encoded in MemoryLocation.cpp w.r.t. access size.)

[reames]

Actually, this patch is slightly wrong. The problem is that argument 0 of memset_pattern16 should be Mod, not ModRef. We don't have the equivalent of a writeonly attribute today.

I still think the rest of the change is worthwhile, but I will have to leave the Mod return special case in BasicAA.

lib/Analysis/BasicAliasAnalysis.cpp

do {
     return AAResultBase::pointsToConstantMemory(Loc, OrLocal);
 
   } while (!Worklist.empty() && --MaxLookup);
 
   Visited.clear();
   return Worklist.empty();
 }
 
-// FIXME: This code is duplicated with MemoryLocation and should be hoisted to
-// some common utility location.
-static bool isMemsetPattern16(const Function *MS,
-                              const TargetLibraryInfo &TLI) {
-  if (TLI.has(LibFunc::memset_pattern16) &&
-      MS->getName() == "memset_pattern16") {
-    FunctionType *MemsetType = MS->getFunctionType();
-    if (!MemsetType->isVarArg() && MemsetType->getNumParams() == 3 &&
-        isa<PointerType>(MemsetType->getParamType(0)) &&
-        isa<PointerType>(MemsetType->getParamType(1)) &&
-        isa<IntegerType>(MemsetType->getParamType(2)))
-      return true;
-  }
-
-  return false;
-}
-
 /// Returns the behavior when calling the given call site.
 FunctionModRefBehavior BasicAAResult::getModRefBehavior(ImmutableCallSite CS) {
   if (CS.doesNotAccessMemory())
     // Can't do better than this.
     return FMRB_DoesNotAccessMemory;
 
   FunctionModRefBehavior Min = FMRB_UnknownModRefBehavior;
 
FunctionModRefBehavior BasicAAResult::getModRefBehavior(const Function *F) {
 
   // If the function declares it only reads memory, go with that.
   if (F->onlyReadsMemory())
     Min = FMRB_OnlyReadsMemory;
 
   if (F->onlyAccessesArgMemory())
     Min = FunctionModRefBehavior(Min & FMRB_OnlyAccessesArgumentPointees);
 
-  if (isMemsetPattern16(F, TLI))
-    Min = FMRB_OnlyAccessesArgumentPointees;
-
   // Otherwise be conservative.
   return FunctionModRefBehavior(AAResultBase::getModRefBehavior(F) & Min);
 }
 
 ModRefInfo BasicAAResult::getArgModRefInfo(ImmutableCallSite CS,
                                            unsigned ArgIdx) {
   if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(CS.getInstruction()))
     switch (II->getIntrinsicID()) {
     default:
       break;
     case Intrinsic::memset:
     case Intrinsic::memcpy:
     case Intrinsic::memmove:
       assert((ArgIdx == 0 || ArgIdx == 1) &&
              "Invalid argument index for memory intrinsic");
       return ArgIdx ? MRI_Ref : MRI_Mod;
     }
 
-  // We can bound the aliasing properties of memset_pattern16 just as we can
-  // for memcpy/memset.  This is particularly important because the
-  // LoopIdiomRecognizer likes to turn loops into calls to memset_pattern16
-  // whenever possible.
-  if (CS.getCalledFunction() &&
-      isMemsetPattern16(CS.getCalledFunction(), TLI)) {
-    assert((ArgIdx == 0 || ArgIdx == 1) &&
-           "Invalid argument index for memset_pattern16");
-    return ArgIdx ? MRI_Ref : MRI_Mod;
-  }
-  // FIXME: Handle memset_pattern4 and memset_pattern8 also.
-
   if (CS.paramHasAttr(ArgIdx + 1, Attribute::ReadOnly))
     return MRI_Ref;
 
   if (CS.paramHasAttr(ArgIdx + 1, Attribute::ReadNone))
     return MRI_NoModRef;
 
   return AAResultBase::getArgModRefInfo(CS, ArgIdx);
 }

lib/Transforms/IPO/InferFunctionAttrs.cpp

 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "inferattrs"
 
 STATISTIC(NumReadNone, "Number of functions inferred as readnone");
 STATISTIC(NumReadOnly, "Number of functions inferred as readonly");
+STATISTIC(NumArgMemOnly, "Number of functions inferred as argmemonly");
 STATISTIC(NumNoUnwind, "Number of functions inferred as nounwind");
 STATISTIC(NumNoCapture, "Number of arguments inferred as nocapture");
 STATISTIC(NumReadOnlyArg, "Number of arguments inferred as readonly");
 STATISTIC(NumNoAlias, "Number of function returns inferred as noalias");
 STATISTIC(NumNonNull, "Number of function returns inferred as nonnull returns");
 
 static bool setDoesNotAccessMemory(Function &F) {
   if (F.doesNotAccessMemory())
     return false;
   F.setDoesNotAccessMemory();
   ++NumReadNone;
   return true;
 }
 
 static bool setOnlyReadsMemory(Function &F) {
   if (F.onlyReadsMemory())
     return false;
   F.setOnlyReadsMemory();
   ++NumReadOnly;
   return true;
 }
 
+static bool setOnlyAccessesArgMemory(Function &F) {
+  if (F.onlyAccessesArgMemory())
+    return false;
+  F.setOnlyAccessesArgMemory ();
+  ++NumArgMemOnly;
+  return true;
+}
+
+
 static bool setDoesNotThrow(Function &F) {
   if (F.doesNotThrow())
     return false;
   F.setDoesNotThrow();
   ++NumNoUnwind;
   return true;
 }
 
static bool inferPrototypeAttributes(Function &F,
   case LibFunc::msvc_new_array_longlong: // new[](unsigned long long)
     if (FTy->getNumParams() != 1)
       return false;
     // Operator new always returns a nonnull noalias pointer
     Changed |= setNonNull(F, AttributeSet::ReturnIndex);
     Changed |= setDoesNotAlias(F, AttributeSet::ReturnIndex);
     return Changed;
 
+  //TODO: add LibFunc entries for:
+  //case LibFunc::memset_pattern4:
+  //case LibFunc::memset_pattern8:
+  case LibFunc::memset_pattern16:
+    if (FTy->isVarArg() || FTy->getNumParams() != 3 ||
+        !isa<PointerType>(FTy->getParamType(0)) ||
+        !isa<PointerType>(FTy->getParamType(1)) ||
+        !isa<IntegerType>(FTy->getParamType(2)))
+      return false;
+
+    Changed |= setOnlyAccessesArgMemory(F);
+    Changed |= setOnlyReadsMemory(F, 2);
+    return Changed;
+
   default:
     // FIXME: It'd be really nice to cover all the library functions we're
     // aware of here.
     return false;
   }
 }
 
 static bool inferAllPrototypeAttributes(Module &M,

test/Analysis/BasicAA/memset_pattern.ll

entry:
   store i32 1, i32* @z
   tail call void @memset_pattern16(i8* bitcast (i32* @y to i8*), i8* bitcast (i32* @x to i8*), i64 4) nounwind
 ; CHECK-NOT: load
   %l = load i32, i32* @z
 ; CHECK: ret i32 1
   ret i32 %l
 }
 
-declare void @memset_pattern16(i8*, i8*, i64)
+declare void @memset_pattern16(i8*, i8* readonly, i64) argmemonly

test/Transforms/InferFunctionAttrs/annotate.ll

 ; CHECK: declare i32 @strcpy(...)
 
 ; operator new routines
 declare i8* @_Znwj(i64)
 ; CHECK: declare noalias nonnull i8* @_Znwj(i64)
 declare i8* @_Znwm(i64)
 ; CHECK: declare noalias nonnull i8* @_Znwm(i64)
 
+declare void @memset_pattern16(i8*, i8*, i64)
+; CHECK: declare void @memset_pattern16(i8*, i8*, i64)
+; CHECK-POSIX: declare void @memset_pattern16(i8*, i8* readonly, i64) [[G2:#[0-9]+]]
+
 declare i32 @gettimeofday(i8*, i8*)
 ; CHECK-POSIX: declare i32 @gettimeofday(i8* nocapture, i8* nocapture) [[G0:#[0-9]+]]
 
 ; CHECK: attributes [[G0]] = { nounwind }
 ; CHECK: attributes [[G1]] = { nounwind readonly }
 ; CHECK-POSIX: attributes [[G0]] = { nounwind }
+; CHECK-POSIX: attributes [[G2]] = { argmemonly }