Skip to content

Commit 5f026b6

Browse files
committed
[DependenceAnalysis] Dependecies for loads marked with "ivnariant.load" should not be shared with general accesses. Fix for https://bugs.llvm.org/show_bug.cgi?id=42151
Summary: Dependence anlysis has a mechanism to cache results. Thus for particular memory access the cache keep track of side effects in basic blocks. The problem is that for invariant loads dependepce analysis legally ignores many dependencies due to a special semantic rules for such loads. But later results calculated for invariant load retrived from the cache for general case acceses. As a result we have wrong dependence information causing GVN to do illegal transformation. Fixes, T42151. Proposed solution is to disable caching of invariant loads. I think such loads a pretty rare and it doesn't make sense to extend caching mechanism for them. Reviewers: reames, chandlerc, skatkov, morisset, jdoerfert Reviewed By: reames Subscribers: hiraditya, test, jdoerfert, lebedev.ri, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D64405
1 parent b6d7bbf commit 5f026b6

File tree

1 file changed

+19
-6
lines changed

1 file changed

+19
-6
lines changed

llvm/lib/Analysis/MemoryDependenceAnalysis.cpp

+19-6
Original file line numberDiff line numberDiff line change
@@ -979,6 +979,11 @@ MemDepResult MemoryDependenceResults::GetNonLocalInfoForBlock(
979979
Instruction *QueryInst, const MemoryLocation &Loc, bool isLoad,
980980
BasicBlock *BB, NonLocalDepInfo *Cache, unsigned NumSortedEntries) {
981981

982+
bool isInvariantLoad = false;
983+
984+
if (LoadInst *LI = dyn_cast_or_null<LoadInst>(QueryInst))
985+
isInvariantLoad = LI->getMetadata(LLVMContext::MD_invariant_load);
986+
982987
// Do a binary search to see if we already have an entry for this block in
983988
// the cache set. If so, find it.
984989
NonLocalDepInfo::iterator Entry = std::upper_bound(
@@ -990,6 +995,13 @@ MemDepResult MemoryDependenceResults::GetNonLocalInfoForBlock(
990995
if (Entry != Cache->begin() + NumSortedEntries && Entry->getBB() == BB)
991996
ExistingResult = &*Entry;
992997

998+
// Use cached result for invariant load only if there is no dependency for non
999+
// invariant load. In this case invariant load can not have any dependency as
1000+
// well.
1001+
if (ExistingResult && isInvariantLoad &&
1002+
!ExistingResult->getResult().isNonFuncLocal())
1003+
ExistingResult = nullptr;
1004+
9931005
// If we have a cached entry, and it is non-dirty, use it as the value for
9941006
// this dependency.
9951007
if (ExistingResult && !ExistingResult->getResult().isDirty()) {
@@ -1018,6 +1030,10 @@ MemDepResult MemoryDependenceResults::GetNonLocalInfoForBlock(
10181030
MemDepResult Dep =
10191031
getPointerDependencyFrom(Loc, isLoad, ScanPos, BB, QueryInst);
10201032

1033+
// Don't cache results for invariant load.
1034+
if (isInvariantLoad)
1035+
return Dep;
1036+
10211037
// If we had a dirty entry for the block, update it. Otherwise, just add
10221038
// a new entry.
10231039
if (ExistingResult)
@@ -1454,22 +1470,19 @@ bool MemoryDependenceResults::getNonLocalPointerDepFromBB(
14541470
if (SkipFirstBlock)
14551471
return false;
14561472

1457-
bool foundBlock = false;
14581473
for (NonLocalDepEntry &I : llvm::reverse(*Cache)) {
14591474
if (I.getBB() != BB)
14601475
continue;
14611476

14621477
assert((GotWorklistLimit || I.getResult().isNonLocal() ||
14631478
!DT.isReachableFromEntry(BB)) &&
14641479
"Should only be here with transparent block");
1465-
foundBlock = true;
14661480
I.setResult(MemDepResult::getUnknown());
1467-
Result.push_back(
1468-
NonLocalDepResult(I.getBB(), I.getResult(), Pointer.getAddr()));
14691481
break;
14701482
}
1471-
(void)foundBlock; (void)GotWorklistLimit;
1472-
assert((foundBlock || GotWorklistLimit) && "Current block not in cache?");
1483+
// Go ahead and report unknown dependence.
1484+
Result.push_back(
1485+
NonLocalDepResult(BB, MemDepResult::getUnknown(), Pointer.getAddr()));
14731486
}
14741487

14751488
// Okay, we're done now. If we added new values to the cache, re-sort it.

0 commit comments

Comments
 (0)