fix a miscompilation of tramp3d-v4: when forming a memcpy, we have to make

sure that the loop we're promoting into a memcpy doesn't mutate the input
of the memcpy.  Before we were just checking that the dest of the memcpy
wasn't mod/ref'd by the loop.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@122712 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Transforms/Scalar/LoopIdiomRecognize.cpp

@@ -175,6 +175,10 @@ bool LoopIdiomRecognize::runOnLoop(Loop *L, LPPassManager &LPM) {
   SmallVector<BasicBlock*, 8> ExitBlocks;
   CurLoop->getUniqueExitBlocks(ExitBlocks);
 
+  DEBUG(dbgs() << "loop-idiom Scanning: F["
+               << L->getHeader()->getParent()->getName()
+               << "] Loop %" << L->getHeader()->getName() << "\n");
+  
   bool MadeChange = false;
   // Scan all the blocks in the loop that are not in subloops.
   for (Loop::block_iterator BI = L->block_begin(), E = L->block_end(); BI != E;
@@ -200,9 +204,6 @@ bool LoopIdiomRecognize::runOnLoopBlock(BasicBlock *BB, const SCEV *BECount,
     if (!DT->dominates(BB, ExitBlocks[i]))
       return false;
   
-  DEBUG(dbgs() << "loop-idiom Scanning: F[" << BB->getParent()->getName()
-        << "] Loop %" << BB->getName() << "\n");
-  
   bool MadeChange = false;
   for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ) {
     // Look for store instructions, which may be memsets.
@@ -276,10 +277,11 @@ bool LoopIdiomRecognize::processLoopStore(StoreInst *SI, const SCEV *BECount) {
   return false;
 }
 
-/// mayLoopModRefLocation - Return true if the specified loop might do a load or
-/// store to the same location that the specified store could store to, which is
-/// a loop-strided access. 
-static bool mayLoopModRefLocation(Value *Ptr, Loop *L, const SCEV *BECount,
+/// mayLoopAccessLocation - Return true if the specified loop might access the
+/// specified pointer location, which is a loop-strided access.  The 'Access'
+/// argument specifies what the verboten forms of access are (read or write).
+static bool mayLoopAccessLocation(Value *Ptr,AliasAnalysis::ModRefResult Access,
+                                  Loop *L, const SCEV *BECount,
                                   unsigned StoreSize, AliasAnalysis &AA,
                                   StoreInst *IgnoredStore) {
   // Get the location that may be stored across the loop.  Since the access is
@@ -302,7 +304,7 @@ static bool mayLoopModRefLocation(Value *Ptr, Loop *L, const SCEV *BECount,
        ++BI)
     for (BasicBlock::iterator I = (*BI)->begin(), E = (*BI)->end(); I != E; ++I)
       if (&*I != IgnoredStore &&
-          AA.getModRefInfo(I, StoreLoc) != AliasAnalysis::NoModRef)
+          (AA.getModRefInfo(I, StoreLoc) & Access))
         return true;
 
   return false;
@@ -323,7 +325,8 @@ processLoopStoreOfSplatValue(StoreInst *SI, unsigned StoreSize,
   // this into a memset in the loop preheader now if we want.  However, this
   // would be unsafe to do if there is anything else in the loop that may read
   // or write to the aliased location.  Check for an alias.
-  if (mayLoopModRefLocation(SI->getPointerOperand(), CurLoop, BECount,
+  if (mayLoopAccessLocation(SI->getPointerOperand(), AliasAnalysis::ModRef,
+                            CurLoop, BECount,
                             StoreSize, getAnalysis<AliasAnalysis>(), SI))
     return false;
   
@@ -386,10 +389,18 @@ processLoopStoreOfLoopLoad(StoreInst *SI, unsigned StoreSize,
   // Okay, we have a strided store "p[i]" of a loaded value.  We can turn
   // this into a memcpy in the loop preheader now if we want.  However, this
   // would be unsafe to do if there is anything else in the loop that may read
-  // or write to the aliased location (including the load feeding the stores).
+  // or write to the stored location (including the load feeding the stores).
   // Check for an alias.
-  if (mayLoopModRefLocation(SI->getPointerOperand(), CurLoop, BECount,
-                            StoreSize, getAnalysis<AliasAnalysis>(), SI))
+  if (mayLoopAccessLocation(SI->getPointerOperand(), AliasAnalysis::ModRef,
+                            CurLoop, BECount, StoreSize,
+                            getAnalysis<AliasAnalysis>(), SI))
+    return false;
+
+  // For a memcpy, we have to make sure that the input array is not being
+  // mutated by the loop.
+  if (mayLoopAccessLocation(LI->getPointerOperand(), AliasAnalysis::Mod,
+                            CurLoop, BECount, StoreSize,
+                            getAnalysis<AliasAnalysis>(), SI))
     return false;
   
   // Okay, everything looks good, insert the memcpy.

test/Transforms/LoopIdiom/basic.ll

@@ -207,3 +207,36 @@ for.end:                                          ; preds = %for.body, %entry
 ; CHECK: store i64 0, i64* %PI
 }
 
+declare i8* @external(i8*)
+
+;; This cannot be transformed into a memcpy, because the read-from location is
+;; mutated by the loop.
+define void @test9(i64 %Size) nounwind ssp {
+bb.nph:
+  %Base = alloca i8, i32 10000
+  %Dest = alloca i8, i32 10000
+  
+  %BaseAlias = call i8* @external(i8* %Base)
+  br label %for.body
+
+for.body:                                         ; preds = %bb.nph, %for.body
+  %indvar = phi i64 [ 0, %bb.nph ], [ %indvar.next, %for.body ]
+  %I.0.014 = getelementptr i8* %Base, i64 %indvar
+  %DestI = getelementptr i8* %Dest, i64 %indvar
+  %V = load i8* %I.0.014, align 1
+  store i8 %V, i8* %DestI, align 1
+
+  ;; This store can clobber the input.
+  store i8 4, i8* %BaseAlias
+ 
+  %indvar.next = add i64 %indvar, 1
+  %exitcond = icmp eq i64 %indvar.next, %Size
+  br i1 %exitcond, label %for.end, label %for.body
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+; CHECK: @test9
+; CHECK-NOT: llvm.memcpy
+; CHECK: ret void
+}
+