[MemorySSA] Don't bail on phi starting access

[nikic]

Summary

When calling getClobberingMemoryAccess() with MemoryLocation on a MemoryPHI starting access, the walker currently immediately bails and returns the starting access. This makes sense for the API that does not accept a location (as we wouldn't know what clobber we should be checking for), but doesn't make sense for the MemoryLocation-based API. This means that it can't look through a MemoryPHI if it's the starting access, but can if there is one more non-clobbering def in between. This patch removes the limitation.

[nikic]

llvm/lib/Analysis/MemorySSA.cpp

2405

The code previously fetched the defining access for MemoryUses, but imho this doesn't make sense for this API, which accepts a defining access as the starting point, unlike the other one.

[asbirlea]

Your point is valid and the patch looks good.

llvm/lib/Analysis/MemorySSA.cpp

 /// the MemoryAccess that actually clobbers Loc.
 ///
 /// \returns our clobbering memory access
 template <typename AliasAnalysisType>
 MemoryAccess *
 MemorySSA::ClobberWalkerBase<AliasAnalysisType>::getClobberingMemoryAccessBase(
     MemoryAccess *StartingAccess, const MemoryLocation &Loc,
     unsigned &UpwardWalkLimit) {
-  if (isa<MemoryPhi>(StartingAccess))
-    return StartingAccess;
+  assert(!isa<MemoryUse>(StartingAccess) && "Use cannot be defining access");
 
-  auto *StartingUseOrDef = cast<MemoryUseOrDef>(StartingAccess);
+  Instruction *I = nullptr;
+  if (auto *StartingUseOrDef = dyn_cast<MemoryUseOrDef>(StartingAccess)) {
     if (MSSA->isLiveOnEntryDef(StartingUseOrDef))
       return StartingUseOrDef;
 
-  Instruction *I = StartingUseOrDef->getMemoryInst();
+    I = StartingUseOrDef->getMemoryInst();
 
-  // Conservatively, fences are always clobbers, so don't perform the walk if we
-  // hit a fence.
+    // Conservatively, fences are always clobbers, so don't perform the walk if
+    // we hit a fence.
     if (!isa<CallBase>(I) && I->isFenceLike())
       return StartingUseOrDef;
+  }
 
   UpwardsMemoryQuery Q;
-  Q.OriginalAccess = StartingUseOrDef;
+  Q.OriginalAccess = StartingAccess;
   Q.StartingLoc = Loc;
   Q.Inst = nullptr;
   Q.IsCall = false;
 
   // Unlike the other function, do not walk to the def of a def, because we are
   // handed something we already believe is the clobbering access.
   // We never set SkipSelf to true in Q in this method.
-  MemoryAccess *DefiningAccess = isa<MemoryUse>(StartingUseOrDef)
-                                     ? StartingUseOrDef->getDefiningAccess()
-                                     : StartingUseOrDef;
-
   MemoryAccess *Clobber =
-      Walker.findClobber(DefiningAccess, Q, UpwardWalkLimit);
-  LLVM_DEBUG(dbgs() << "Starting Memory SSA clobber for " << *I << " is ");
-  LLVM_DEBUG(dbgs() << *StartingUseOrDef << "\n");
-  LLVM_DEBUG(dbgs() << "Final Memory SSA clobber for " << *I << " is ");
-  LLVM_DEBUG(dbgs() << *Clobber << "\n");
+      Walker.findClobber(StartingAccess, Q, UpwardWalkLimit);
+  LLVM_DEBUG({
+    dbgs() << "Clobber starting at access " << *StartingAccess << "\n";
+    if (I)
+      dbgs() << "  for instruction " << *I << "\n";
+    dbgs() << "  is " << *Clobber << "\n";
+  });
   return Clobber;
 }
 
 template <typename AliasAnalysisType>
 MemoryAccess *
 MemorySSA::ClobberWalkerBase<AliasAnalysisType>::getClobberingMemoryAccessBase(
     MemoryAccess *MA, unsigned &UpwardWalkLimit, bool SkipSelf) {
   auto *StartingAccess = dyn_cast<MemoryUseOrDef>(MA);

llvm/test/Transforms/MemCpyOpt/memcpy-in-loop.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -memcpyopt < %s | FileCheck %s
 
 define void @test_copy_uninit([1000 x [1000 x i32]]* %arg) {
 ; CHECK-LABEL: @test_copy_uninit(
 ; CHECK-NEXT:  start:
 ; CHECK-NEXT:    [[ALLOCA:%.*]] = alloca [1000 x i32], align 4
 ; CHECK-NEXT:    [[ALLOCA_I8:%.*]] = bitcast [1000 x i32]* [[ALLOCA]] to i8*
 ; CHECK-NEXT:    [[BEGIN:%.*]] = getelementptr inbounds [1000 x [1000 x i32]], [1000 x [1000 x i32]]* [[ARG:%.*]], i64 0, i64 0
 ; CHECK-NEXT:    [[END:%.*]] = getelementptr inbounds [1000 x [1000 x i32]], [1000 x [1000 x i32]]* [[ARG]], i64 0, i64 1000
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    [[CURRENT:%.*]] = phi [1000 x i32]* [ [[BEGIN]], [[START:%.*]] ], [ [[NEXT:%.*]], [[LOOP]] ]
 ; CHECK-NEXT:    [[CURRENT_I8:%.*]] = bitcast [1000 x i32]* [[CURRENT]] to i8*
-; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 4 dereferenceable(4000) [[CURRENT_I8]], i8* nonnull align 4 dereferenceable(4000) [[ALLOCA_I8]], i64 4000, i1 false)
 ; CHECK-NEXT:    [[NEXT]] = getelementptr inbounds [1000 x i32], [1000 x i32]* [[CURRENT]], i64 1
 ; CHECK-NEXT:    [[COND:%.*]] = icmp eq [1000 x i32]* [[NEXT]], [[END]]
 ; CHECK-NEXT:    br i1 [[COND]], label [[EXIT:%.*]], label [[LOOP]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    ret void
 ;
 start:
   %alloca = alloca [1000 x i32], align 4

 ; CHECK-NEXT:    [[ALLOCA_I8:%.*]] = bitcast [1000 x i32]* [[ALLOCA]] to i8*
 ; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* nonnull align 4 dereferenceable(4000) [[ALLOCA_I8]], i8 0, i64 4000, i1 false)
 ; CHECK-NEXT:    [[BEGIN:%.*]] = getelementptr inbounds [1000 x [1000 x i32]], [1000 x [1000 x i32]]* [[ARG:%.*]], i64 0, i64 0
 ; CHECK-NEXT:    [[END:%.*]] = getelementptr inbounds [1000 x [1000 x i32]], [1000 x [1000 x i32]]* [[ARG]], i64 0, i64 1000
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    [[CURRENT:%.*]] = phi [1000 x i32]* [ [[BEGIN]], [[START:%.*]] ], [ [[NEXT:%.*]], [[LOOP]] ]
 ; CHECK-NEXT:    [[CURRENT_I8:%.*]] = bitcast [1000 x i32]* [[CURRENT]] to i8*
-; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 4 dereferenceable(4000) [[CURRENT_I8]], i8* nonnull align 4 dereferenceable(4000) [[ALLOCA_I8]], i64 4000, i1 false)
+; CHECK-NEXT:    call void @llvm.memset.p0i8.i64(i8* align 4 [[CURRENT_I8]], i8 0, i64 4000, i1 false)
 ; CHECK-NEXT:    [[NEXT]] = getelementptr inbounds [1000 x i32], [1000 x i32]* [[CURRENT]], i64 1
 ; CHECK-NEXT:    [[COND:%.*]] = icmp eq [1000 x i32]* [[NEXT]], [[END]]
 ; CHECK-NEXT:    br i1 [[COND]], label [[EXIT:%.*]], label [[LOOP]]
 ; CHECK:       exit:
 ; CHECK-NEXT:    ret void
 ;
 start:
   %alloca = alloca [1000 x i32], align 4