Fix issues required to remove memset/memcpy/memmove special casing from AliasSetTracker

[reames]

Summary

The heart of this change is an NFC which removes dedicated handling in AST for handling mem* intrinsics in favour of the recently added generic argmemonly handling.

However, doing this exposed two deeper issues which are definitely not NFC:

1. We'd never sunk the handling for the atomic.elementwise variants down into MemoryLocation (which is the codepath used by the generic argmemonly work)
2. We weren't treating memmove as being writeonly in it's first argument. This is really the change which concerns me. (Note: memmove allows aliasing src and dest, but that doesn't allow the we can read *through* the dest argument which is how the parameter attributes are specified.)

Together, these two make the change mildly risky and I figured an extra set or two of eyes was definitely warranted.

[reames]

lib/Analysis/MemoryLocation.cpp

126

Realized I'd missed the atomic memset case. I added new tests (in test/Analysis/AliasSet/memset.ll) which would have caught this and will rebase at some point.

[reames]

Not something I'm actively working on. Will split apart when I get back to this for easier review.

[reames]

No longer actively working on this, may return to it in the future.

include/llvm/Analysis/AliasSetTracker.h

 #include <iterator>
 #include <vector>
 
 namespace llvm {
 
 class AliasSetTracker;
 class BasicBlock;
 class LoadInst;
-class AnyMemSetInst;
-class AnyMemTransferInst;
 class raw_ostream;
 class StoreInst;
 class VAArgInst;
 class Value;
 
 class AliasSet : public ilist_node<AliasSet> {
   friend class AliasSetTracker;
 
public:
   ///
   /// These methods return true if inserting the instruction resulted in the
   /// addition of a new alias set (i.e., the pointer did not alias anything).
   ///
   void add(Value *Ptr, LocationSize Size, const AAMDNodes &AAInfo); // Add a loc
   void add(LoadInst *LI);
   void add(StoreInst *SI);
   void add(VAArgInst *VAAI);
-  void add(AnyMemSetInst *MSI);
-  void add(AnyMemTransferInst *MTI);
   void add(Instruction *I);       // Dispatch to one of the other add methods...
   void add(BasicBlock &BB);       // Add all instructions in basic block
   void add(const AliasSetTracker &AST); // Add alias relations from another AST
   void addUnknown(Instruction *I);
 
   void clear();
 
   /// Return the alias sets that are active.

include/llvm/IR/Intrinsics.td

def int_memcpy  : Intrinsic<[],
                              [llvm_anyptr_ty, llvm_anyptr_ty, llvm_anyint_ty,
                               llvm_i1_ty],
                             [IntrArgMemOnly, NoCapture<0>, NoCapture<1>,
                              WriteOnly<0>, ReadOnly<1>]>;
 def int_memmove : Intrinsic<[],
                             [llvm_anyptr_ty, llvm_anyptr_ty, llvm_anyint_ty,
                              llvm_i1_ty],
                             [IntrArgMemOnly, NoCapture<0>, NoCapture<1>,
-                             ReadOnly<1>]>;
+                             WriteOnly<0>, ReadOnly<1>]>;
 def int_memset  : Intrinsic<[],
                             [llvm_anyptr_ty, llvm_i8_ty, llvm_anyint_ty,
                              llvm_i1_ty],
                             [IntrArgMemOnly, NoCapture<0>, WriteOnly<0>]>;
 
 // FIXME: Add version of these floating point intrinsics which allow non-default
 // rounding modes and FP exception handling.
 

lib/Analysis/AliasSetTracker.cpp

if (isStrongerThanMonotonic(SI->getOrdering()))
     return addUnknown(SI);
   addPointer(MemoryLocation::get(SI), AliasSet::ModAccess);
 }
 
 void AliasSetTracker::add(VAArgInst *VAAI) {
   addPointer(MemoryLocation::get(VAAI), AliasSet::ModRefAccess);
 }
 
-void AliasSetTracker::add(AnyMemSetInst *MSI) {
-  addPointer(MemoryLocation::getForDest(MSI), AliasSet::ModAccess);
-}
-
-void AliasSetTracker::add(AnyMemTransferInst *MTI) {
-  addPointer(MemoryLocation::getForDest(MTI), AliasSet::ModAccess);
-  addPointer(MemoryLocation::getForSource(MTI), AliasSet::RefAccess);
-}
-
 void AliasSetTracker::addUnknown(Instruction *Inst) {
   if (isa<DbgInfoIntrinsic>(Inst))
     return; // Ignore DbgInfo Intrinsics.
 
   if (auto *II = dyn_cast<IntrinsicInst>(Inst)) {
     // These intrinsics will show up as affecting memory, but they are just
     // markers.
     switch (II->getIntrinsicID()) {

 void AliasSetTracker::add(Instruction *I) {
   // Dispatch to one of the other add methods.
   if (LoadInst *LI = dyn_cast<LoadInst>(I))
     return add(LI);
   if (StoreInst *SI = dyn_cast<StoreInst>(I))
     return add(SI);
   if (VAArgInst *VAAI = dyn_cast<VAArgInst>(I))
     return add(VAAI);
-  if (AnyMemSetInst *MSI = dyn_cast<AnyMemSetInst>(I))
-    return add(MSI);
-  if (AnyMemTransferInst *MTI = dyn_cast<AnyMemTransferInst>(I))
-    return add(MTI);
 
   // Handle all calls with known mod/ref sets genericall
   CallSite CS(I);
   if (CS && CS.onlyAccessesArgMemory()) {
     auto getAccessFromModRef = [](ModRefInfo MRI) {
       if (isRefSet(MRI) && isModSet(MRI))
         return AliasSet::ModRefAccess;
       else if (isModSet(MRI))

lib/Analysis/MemoryLocation.cpp

if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(CS.getInstruction())) {
     const DataLayout &DL = II->getModule()->getDataLayout();
 
     switch (II->getIntrinsicID()) {
     default:
       break;
     case Intrinsic::memset:
     case Intrinsic::memcpy:
     case Intrinsic::memmove:
+    case Intrinsic::memcpy_element_unordered_atomic:
+    case Intrinsic::memmove_element_unordered_atomic:
       assert((ArgIdx == 0 || ArgIdx == 1) &&
              "Invalid argument index for memory intrinsic");
       if (ConstantInt *LenCI = dyn_cast<ConstantInt>(II->getArgOperand(2)))
         return MemoryLocation(Arg, LenCI->getZExtValue(), AATags);
       break;
 
     case Intrinsic::lifetime_start:
     case Intrinsic::lifetime_end:

test/Analysis/AliasSet/memtransfer.ll

entry:
   call void @llvm.memcpy.element.unordered.atomic.p0i8.p0i8.i64(i8* align 1 %b, i8* align 1 %a, i64 1, i32 1)
   call void @llvm.memcpy.element.unordered.atomic.p0i8.p0i8.i64(i8* align 1 %a, i8* align 1 %b, i64 1, i32 1)
   store atomic i8 1, i8* %b unordered, align 1
   ret void
 }
 
 declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i1)
 declare void @llvm.memcpy.element.unordered.atomic.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32)
-declare void @llvm.memmove.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i1)
+declare void @llvm.memmove.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i1)
 declare void @llvm.memmove.element.unordered.atomic.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32)

test/Bitcode/upgrade-memory-intrinsics.ll

; CHECK: call void @llvm.memmove.p0i8.p0i8.i64(i8* %p2, i8* %p3, i64 1000, i1 false)
   call void @llvm.memset.p0i8.i64(i8* %p1, i8 55, i64 100, i32 0, i1 false)
   call void @llvm.memcpy.p0i8.p0i8.i64(i8* %p1, i8* %p2, i64 50, i32 0, i1 false)
   call void @llvm.memmove.p0i8.p0i8.i64(i8* %p2, i8* %p3, i64 1000, i32 0, i1 false)
   ret void
 }
 
 ; CHECK: declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1)
 ; CHECK: declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i1)
-; CHECK: declare void @llvm.memmove.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i1)
+; CHECK: declare void @llvm.memmove.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i1)
 declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i32, i1)
 declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly , i8* nocapture readonly, i64, i32, i1)
 declare void @llvm.memmove.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1)