Skip to content

Commit 4f6b93b

Browse files
author
Robin Morisset
committed
Fix MemoryDependenceAnalysis in cases where QueryInstr is a CmpXchg or a AtomicRMW
Summary: MemoryDependenceAnalysis is currently cautious when the QueryInstr is an atomic load or store, but I forgot to check for atomic cmpxchg/atomicrmw. This patch is a way of fixing that, and making it less brittle (i.e. no risk that I forget another possible kind of atomic, even if the IR ends up changing in the future), by adding a fallback checking mayReadOrWriteFromMemory. Thanks to Philip Reames for finding this bug and suggesting this solution in http://reviews.llvm.org/D4845 Sadly, I don't see how to add a test for this, since the passes depending on MemoryDependenceAnalysis won't trigger for an atomic rmw anyway. Does anyone see a way for testing it? Test Plan: none possible at first sight Reviewers: jfb, reames Subscribers: llvm-commits Differential Revision: http://reviews.llvm.org/D5019 llvm-svn: 216940
1 parent b2325b9 commit 4f6b93b

File tree

1 file changed

+12
-4
lines changed

1 file changed

+12
-4
lines changed

llvm/lib/Analysis/MemoryDependenceAnalysis.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -449,12 +449,16 @@ getPointerDependencyFrom(const AliasAnalysis::Location &MemLoc, bool isLoad,
449449
if (!LI->isUnordered()) {
450450
if (!QueryInst)
451451
return MemDepResult::getClobber(LI);
452-
if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst))
452+
if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst)) {
453453
if (!QueryLI->isSimple())
454454
return MemDepResult::getClobber(LI);
455-
if (auto *QuerySI = dyn_cast<StoreInst>(QueryInst))
455+
} else if (auto *QuerySI = dyn_cast<StoreInst>(QueryInst)) {
456456
if (!QuerySI->isSimple())
457457
return MemDepResult::getClobber(LI);
458+
} else if (QueryInst->mayReadOrWriteMemory()) {
459+
return MemDepResult::getClobber(LI);
460+
}
461+
458462
if (isAtLeastAcquire(LI->getOrdering()))
459463
HasSeenAcquire = true;
460464
}
@@ -529,12 +533,16 @@ getPointerDependencyFrom(const AliasAnalysis::Location &MemLoc, bool isLoad,
529533
if (!SI->isUnordered()) {
530534
if (!QueryInst)
531535
return MemDepResult::getClobber(SI);
532-
if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst))
536+
if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst)) {
533537
if (!QueryLI->isSimple())
534538
return MemDepResult::getClobber(SI);
535-
if (auto *QuerySI = dyn_cast<StoreInst>(QueryInst))
539+
} else if (auto *QuerySI = dyn_cast<StoreInst>(QueryInst)) {
536540
if (!QuerySI->isSimple())
537541
return MemDepResult::getClobber(SI);
542+
} else if (QueryInst->mayReadOrWriteMemory()) {
543+
return MemDepResult::getClobber(SI);
544+
}
545+
538546
if (HasSeenAcquire && isAtLeastRelease(SI->getOrdering()))
539547
return MemDepResult::getClobber(SI);
540548
}

0 commit comments

Comments
 (0)