diff options
| -rw-r--r-- | llvm/lib/Transforms/Scalar/GVNHoist.cpp | 63 | ||||
| -rw-r--r-- | llvm/test/Transforms/GVNHoist/pr30216.ll | 52 |
2 files changed, 28 insertions, 87 deletions
diff --git a/llvm/lib/Transforms/Scalar/GVNHoist.cpp b/llvm/lib/Transforms/Scalar/GVNHoist.cpp index 03fd4872a0a..8b2164c5071 100644 --- a/llvm/lib/Transforms/Scalar/GVNHoist.cpp +++ b/llvm/lib/Transforms/Scalar/GVNHoist.cpp @@ -329,56 +329,49 @@ private: return I1DFS < I2DFS; } - // Return true when there are memory uses of Def in BB. - bool hasMemoryUseOnPath(const Instruction *NewPt, MemoryDef *Def, const BasicBlock *BB) { - const Instruction *OldPt = Def->getMemoryInst(); + // Return true when there are users of Def in BB. + bool hasMemoryUseOnPath(MemoryAccess *Def, const BasicBlock *BB, + const Instruction *OldPt) { + const BasicBlock *DefBB = Def->getBlock(); const BasicBlock *OldBB = OldPt->getParent(); - const BasicBlock *NewBB = NewPt->getParent(); - bool ReachedNewPt = false; - MemoryLocation DefLoc = MemoryLocation::get(OldPt); - const MemorySSA::AccessList *Acc = MSSA->getBlockAccesses(BB); - if (!Acc) - return false; + for (User *U : Def->users()) + if (auto *MU = dyn_cast<MemoryUse>(U)) { + // FIXME: MU->getBlock() does not get updated when we move the instruction. + BasicBlock *UBB = MU->getMemoryInst()->getParent(); + // Only analyze uses in BB. + if (BB != UBB) + continue; - for (const MemoryAccess &MA : *Acc) { - auto *MU = dyn_cast<MemoryUse>(&MA); - if (!MU) - continue; + // A use in the same block as the Def is on the path. + if (UBB == DefBB) { + assert(MSSA->locallyDominates(Def, MU) && "def not dominating use"); + return true; + } - // Do not check whether MU aliases Def when MU occurs after OldPt. - if (BB == OldBB && firstInBB(OldPt, MU->getMemoryInst())) - break; + if (UBB != OldBB) + return true; - // Do not check whether MU aliases Def when MU occurs before NewPt. - if (BB == NewBB) { - if (!ReachedNewPt) { - if (firstInBB(MU->getMemoryInst(), NewPt)) - continue; - ReachedNewPt = true; - } + // It is only harmful to hoist when the use is before OldPt. + if (firstInBB(MU->getMemoryInst(), OldPt)) + return true; } - if (!AA->isNoAlias(DefLoc, MemoryLocation::get(MU->getMemoryInst()))) - return true; - } - return false; } // Return true when there are exception handling or loads of memory Def - // between Def and NewPt. This function is only called for stores: Def is - // the MemoryDef of the store to be hoisted. + // between OldPt and NewPt. // Decrement by 1 NBBsOnAllPaths for each block between HoistPt and BB, and // return true when the counter NBBsOnAllPaths reaces 0, except when it is // initialized to -1 which is unlimited. - bool hasEHOrLoadsOnPath(const Instruction *NewPt, MemoryDef *Def, - int &NBBsOnAllPaths) { + bool hasEHOrLoadsOnPath(const Instruction *NewPt, const Instruction *OldPt, + MemoryAccess *Def, int &NBBsOnAllPaths) { const BasicBlock *NewBB = NewPt->getParent(); - const BasicBlock *OldBB = Def->getBlock(); + const BasicBlock *OldBB = OldPt->getParent(); assert(DT->dominates(NewBB, OldBB) && "invalid path"); - assert(DT->dominates(Def->getDefiningAccess()->getBlock(), NewBB) && + assert(DT->dominates(Def->getBlock(), NewBB) && "def does not dominate new hoisting point"); // Walk all basic blocks reachable in depth-first iteration on the inverse @@ -397,7 +390,7 @@ private: return true; // Check that we do not move a store past loads. - if (hasMemoryUseOnPath(NewPt, Def, *I)) + if (hasMemoryUseOnPath(Def, *I, OldPt)) return true; // Stop walk once the limit is reached. @@ -480,7 +473,7 @@ private: // Check for unsafe hoistings due to side effects. if (K == InsKind::Store) { - if (hasEHOrLoadsOnPath(NewPt, dyn_cast<MemoryDef>(U), NBBsOnAllPaths)) + if (hasEHOrLoadsOnPath(NewPt, OldPt, D, NBBsOnAllPaths)) return false; } else if (hasEHOnPath(NewBB, OldBB, NBBsOnAllPaths)) return false; diff --git a/llvm/test/Transforms/GVNHoist/pr30216.ll b/llvm/test/Transforms/GVNHoist/pr30216.ll deleted file mode 100644 index b2ce338f8ae..00000000000 --- a/llvm/test/Transforms/GVNHoist/pr30216.ll +++ /dev/null @@ -1,52 +0,0 @@ -; RUN: opt -S -gvn-hoist < %s | FileCheck %s - -; Make sure the two stores @B do not get hoisted past the load @B. - -; CHECK-LABEL: define i8* @Foo -; CHECK: store -; CHECK: store -; CHECK: load -; CHECK: store - -@A = external global i8 -@B = external global i8* - -define i8* @Foo() { - store i8 0, i8* @A - br i1 undef, label %if.then, label %if.else - -if.then: - store i8* null, i8** @B - ret i8* null - -if.else: - %1 = load i8*, i8** @B - store i8* null, i8** @B - ret i8* %1 -} - -; Make sure the two stores @B do not get hoisted past the store @GlobalVar. - -; CHECK-LABEL: define i8* @Fun -; CHECK: store -; CHECK: store -; CHECK: store -; CHECK: store -; CHECK: load - -@GlobalVar = internal global i8 0 - -define i8* @Fun() { - store i8 0, i8* @A - br i1 undef, label %if.then, label %if.else - -if.then: - store i8* null, i8** @B - ret i8* null - -if.else: - store i8 0, i8* @GlobalVar - store i8* null, i8** @B - %1 = load i8*, i8** @B - ret i8* %1 -}
\ No newline at end of file |
