diff options
author | Benjamin Kramer <benny.kra@googlemail.com> | 2012-10-21 15:03:07 +0000 |
---|---|---|
committer | Benjamin Kramer <benny.kra@googlemail.com> | 2012-10-21 15:03:07 +0000 |
commit | 5c6e9ae14e012bfe1fdf0e19957922d3c4d85670 (patch) | |
tree | e8a0f8e874ee1e924d23af4e0601267968409aec /lib/Transforms | |
parent | bb950854acbb5966875763eaae7ab58e48e4f5a9 (diff) | |
download | external_llvm-5c6e9ae14e012bfe1fdf0e19957922d3c4d85670.zip external_llvm-5c6e9ae14e012bfe1fdf0e19957922d3c4d85670.tar.gz external_llvm-5c6e9ae14e012bfe1fdf0e19957922d3c4d85670.tar.bz2 |
LoopIdiom: Replace custom dependence analysis with LoopDependenceAnalysis.
Requires a lot less code and complexity on loop-idiom's side and the more
precise analysis can catch more cases, like the one I included as a test case.
This also fixes the edge-case miscompilation from PR9481. I'm not entirely
sure that all cases are handled that the old checks handled but LDA will
certainly become smarter in the future.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@166390 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'lib/Transforms')
-rw-r--r-- | lib/Transforms/Scalar/LoopIdiomRecognize.cpp | 100 |
1 files changed, 26 insertions, 74 deletions
diff --git a/lib/Transforms/Scalar/LoopIdiomRecognize.cpp b/lib/Transforms/Scalar/LoopIdiomRecognize.cpp index a44e798..5c398a8 100644 --- a/lib/Transforms/Scalar/LoopIdiomRecognize.cpp +++ b/lib/Transforms/Scalar/LoopIdiomRecognize.cpp @@ -48,6 +48,7 @@ #include "llvm/Module.h" #include "llvm/ADT/Statistic.h" #include "llvm/Analysis/AliasAnalysis.h" +#include "llvm/Analysis/LoopDependenceAnalysis.h" #include "llvm/Analysis/LoopPass.h" #include "llvm/Analysis/ScalarEvolutionExpander.h" #include "llvm/Analysis/ScalarEvolutionExpressions.h" @@ -106,6 +107,8 @@ namespace { AU.addPreserved<AliasAnalysis>(); AU.addRequired<ScalarEvolution>(); AU.addPreserved<ScalarEvolution>(); + AU.addRequired<LoopDependenceAnalysis>(); + AU.addPreserved<LoopDependenceAnalysis>(); AU.addPreserved<DominatorTree>(); AU.addRequired<DominatorTree>(); AU.addRequired<TargetLibraryInfo>(); @@ -122,6 +125,7 @@ INITIALIZE_PASS_DEPENDENCY(LoopSimplify) INITIALIZE_PASS_DEPENDENCY(LCSSA) INITIALIZE_PASS_DEPENDENCY(ScalarEvolution) INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfo) +INITIALIZE_PASS_DEPENDENCY(LoopDependenceAnalysis) INITIALIZE_AG_DEPENDENCY(AliasAnalysis) INITIALIZE_PASS_END(LoopIdiomRecognize, "loop-idiom", "Recognize loop idioms", false, false) @@ -163,15 +167,6 @@ static void deleteDeadInstruction(Instruction *I, ScalarEvolution &SE, } while (!NowDeadInsts.empty()); } -/// deleteIfDeadInstruction - If the specified value is a dead instruction, -/// delete it and any recursively used instructions. -static void deleteIfDeadInstruction(Value *V, ScalarEvolution &SE, - const TargetLibraryInfo *TLI) { - if (Instruction *I = dyn_cast<Instruction>(V)) - if (isInstructionTriviallyDead(I, TLI)) - deleteDeadInstruction(I, SE, TLI); -} - bool LoopIdiomRecognize::runOnLoop(Loop *L, LPPassManager &LPM) { CurLoop = L; @@ -368,35 +363,16 @@ processLoopMemSet(MemSetInst *MSI, const SCEV *BECount) { MSI, Ev, 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, - Instruction *IgnoredStore) { - // Get the location that may be stored across the loop. Since the access is - // strided positively through memory, we say that the modified location starts - // at the pointer and has infinite size. - uint64_t AccessSize = AliasAnalysis::UnknownSize; - - // If the loop iterates a fixed number of times, we can refine the access size - // to be exactly the size of the memset, which is (BECount+1)*StoreSize - if (const SCEVConstant *BECst = dyn_cast<SCEVConstant>(BECount)) - AccessSize = (BECst->getValue()->getZExtValue()+1)*StoreSize; - - // TODO: For this to be really effective, we have to dive into the pointer - // operand in the store. Store to &A[i] of 100 will always return may alias - // with store of &A[100], we need to StoreLoc to be "A" with size of 100, - // which will then no-alias a store to &A[100]. - AliasAnalysis::Location StoreLoc(Ptr, AccessSize); - +/// hasDependence - Uses the LoopDependenceAnalysis to determine whether 'Inst' +/// depends on any other value in the Loop 'L'. +static bool hasDependence(Instruction *Inst, Loop *L, + LoopDependenceAnalysis &LDA) { for (Loop::block_iterator BI = L->block_begin(), E = L->block_end(); BI != E; ++BI) for (BasicBlock::iterator I = (*BI)->begin(), E = (*BI)->end(); I != E; ++I) - if (&*I != IgnoredStore && - (AA.getModRefInfo(I, StoreLoc) & Access)) + if (&*I != Inst && I->mayReadOrWriteMemory() && + (I->mayWriteToMemory() || Inst->mayWriteToMemory()) && + LDA.depends(Inst, I)) return true; return false; @@ -474,6 +450,11 @@ processLoopStridedStore(Value *DestPtr, unsigned StoreSize, return false; } + // Make sure the store has no dependencies (i.e. other loads and stores) in + // the loop. + if (hasDependence(TheStore, CurLoop, getAnalysis<LoopDependenceAnalysis>())) + return false; + // The trip count of the loop and the base pointer of the addrec SCEV is // guaranteed to be loop invariant, which means that it should dominate the // header. This allows us to insert code for it in the preheader. @@ -482,25 +463,13 @@ processLoopStridedStore(Value *DestPtr, unsigned StoreSize, SCEVExpander Expander(*SE, "loop-idiom"); // Okay, we have a strided store "p[i]" of a splattable value. We can turn - // 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 any overlap by generating the - // base pointer and checking the region. + // this into a memset in the loop preheader now if we want. unsigned AddrSpace = cast<PointerType>(DestPtr->getType())->getAddressSpace(); Value *BasePtr = Expander.expandCodeFor(Ev->getStart(), Builder.getInt8PtrTy(AddrSpace), Preheader->getTerminator()); - if (mayLoopAccessLocation(BasePtr, AliasAnalysis::ModRef, - CurLoop, BECount, - StoreSize, getAnalysis<AliasAnalysis>(), TheStore)){ - Expander.clear(); - // If we generated new code for the base pointer, clean up. - deleteIfDeadInstruction(BasePtr, *SE, TLI); - return false; - } - // Okay, everything looks good, insert the memset. // The # stored bytes is (BECount+1)*Size. Expand the trip count out to @@ -563,6 +532,14 @@ processLoopStoreOfLoopLoad(StoreInst *SI, unsigned StoreSize, LoadInst *LI = cast<LoadInst>(SI->getValueOperand()); + // Make sure the load and the store have no dependencies (i.e. other loads and + // stores) in the loop. + // FIXME: If we want to form a memmove SI and LI can be dependent but the + // distance must be positive. LDA doesn't provide that info currently. + LoopDependenceAnalysis &LDA = getAnalysis<LoopDependenceAnalysis>(); + if (hasDependence(SI, CurLoop, LDA) || hasDependence(LI, CurLoop, LDA)) + return false; + // The trip count of the loop and the base pointer of the addrec SCEV is // guaranteed to be loop invariant, which means that it should dominate the // header. This allows us to insert code for it in the preheader. @@ -571,41 +548,16 @@ processLoopStoreOfLoopLoad(StoreInst *SI, unsigned StoreSize, SCEVExpander Expander(*SE, "loop-idiom"); // 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 the memory region we're storing to. This includes the load that - // feeds the stores. Check for an alias by generating the base address and - // checking everything. + // this into a memcpy in the loop preheader now if we want. Value *StoreBasePtr = Expander.expandCodeFor(StoreEv->getStart(), Builder.getInt8PtrTy(SI->getPointerAddressSpace()), Preheader->getTerminator()); - - if (mayLoopAccessLocation(StoreBasePtr, AliasAnalysis::ModRef, - CurLoop, BECount, StoreSize, - getAnalysis<AliasAnalysis>(), SI)) { - Expander.clear(); - // If we generated new code for the base pointer, clean up. - deleteIfDeadInstruction(StoreBasePtr, *SE, TLI); - return false; - } - - // For a memcpy, we have to make sure that the input array is not being - // mutated by the loop. Value *LoadBasePtr = Expander.expandCodeFor(LoadEv->getStart(), Builder.getInt8PtrTy(LI->getPointerAddressSpace()), Preheader->getTerminator()); - if (mayLoopAccessLocation(LoadBasePtr, AliasAnalysis::Mod, CurLoop, BECount, - StoreSize, getAnalysis<AliasAnalysis>(), SI)) { - Expander.clear(); - // If we generated new code for the base pointer, clean up. - deleteIfDeadInstruction(LoadBasePtr, *SE, TLI); - deleteIfDeadInstruction(StoreBasePtr, *SE, TLI); - return false; - } - // Okay, everything is safe, we can transform this! |