From: Robin Morisset <morisset@google.com>
Date: Tue, 2 Sep 2014 20:17:52 +0000 (+0000)
Subject: Fix MemoryDependenceAnalysis in cases where QueryInstr is a CmpXchg or a AtomicRMW
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=cc9916444881b06a35ae6402d37a4e38a56dfd52;p=oota-llvm.git

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@216940 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Analysis/MemoryDependenceAnalysis.cpp b/lib/Analysis/MemoryDependenceAnalysis.cpp
index aef241840ac..f7180aae69e 100644
--- a/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -449,12 +449,16 @@ getPointerDependencyFrom(const AliasAnalysis::Location &MemLoc, bool isLoad,
       if (!LI->isUnordered()) {
         if (!QueryInst)
           return MemDepResult::getClobber(LI);
-        if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst))
+        if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst)) {
           if (!QueryLI->isSimple())
             return MemDepResult::getClobber(LI);
-        if (auto *QuerySI = dyn_cast<StoreInst>(QueryInst))
+        } else if (auto *QuerySI = dyn_cast<StoreInst>(QueryInst)) {
           if (!QuerySI->isSimple())
             return MemDepResult::getClobber(LI);
+        } else if (QueryInst->mayReadOrWriteMemory()) {
+          return MemDepResult::getClobber(LI);
+        }
+
         if (isAtLeastAcquire(LI->getOrdering()))
           HasSeenAcquire = true;
       }
@@ -529,12 +533,16 @@ getPointerDependencyFrom(const AliasAnalysis::Location &MemLoc, bool isLoad,
       if (!SI->isUnordered()) {
         if (!QueryInst)
           return MemDepResult::getClobber(SI);
-        if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst))
+        if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst)) {
           if (!QueryLI->isSimple())
             return MemDepResult::getClobber(SI);
-        if (auto *QuerySI = dyn_cast<StoreInst>(QueryInst))
+        } else if (auto *QuerySI = dyn_cast<StoreInst>(QueryInst)) {
           if (!QuerySI->isSimple())
             return MemDepResult::getClobber(SI);
+        } else if (QueryInst->mayReadOrWriteMemory()) {
+          return MemDepResult::getClobber(SI);
+        }
+
         if (HasSeenAcquire && isAtLeastRelease(SI->getOrdering()))
           return MemDepResult::getClobber(SI);
       }