From 63f9c3c49ac7011e6366fbec42716f3f70f1beee Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Sun, 2 Jan 2011 21:14:18 +0000 Subject: [PATCH] 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 | 35 +++++++++++++------- test/Transforms/LoopIdiom/basic.ll | 33 ++++++++++++++++++ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/lib/Transforms/Scalar/LoopIdiomRecognize.cpp b/lib/Transforms/Scalar/LoopIdiomRecognize.cpp index 57a502b1d25..49368cb7f7e 100644 --- a/lib/Transforms/Scalar/LoopIdiomRecognize.cpp +++ b/lib/Transforms/Scalar/LoopIdiomRecognize.cpp @@ -175,6 +175,10 @@ bool LoopIdiomRecognize::runOnLoop(Loop *L, LPPassManager &LPM) { SmallVector 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(), 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(), SI)) + if (mayLoopAccessLocation(SI->getPointerOperand(), AliasAnalysis::ModRef, + CurLoop, BECount, StoreSize, + getAnalysis(), 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(), SI)) return false; // Okay, everything looks good, insert the memcpy. diff --git a/test/Transforms/LoopIdiom/basic.ll b/test/Transforms/LoopIdiom/basic.ll index e8e0f38004c..f3b55d7d614 100644 --- a/test/Transforms/LoopIdiom/basic.ll +++ b/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 +} + -- 2.34.1