Fix MemoryDependenceAnalysis in cases where QueryInstr is a CmpXchg or a AtomicRMW

[morisset]

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?

[morisset]

Rebasing on trunk.

[reames]

LGTM

lib/Analysis/MemoryDependenceAnalysis.cpp

426

nitpick: } else if (c) {

[morisset]

Fix the formatting

Can you accept the revision if it looks good to you ?
Thanks.

[jfb]

lgtm+accept

[morisset]

Closed by commit rL216940 (authored by @morisset).

lib/Analysis/MemoryDependenceAnalysis.cpp

if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
         if (!QueryInst)
           return MemDepResult::getClobber(LI);
         if (LI->getOrdering() != Monotonic)
           return MemDepResult::getClobber(LI);
         if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst))
           if (!QueryLI->isSimple())
             return MemDepResult::getClobber(LI);
         if (auto *QuerySI = dyn_cast<StoreInst>(QueryInst))
           if (!QuerySI->isSimple())
             return MemDepResult::getClobber(LI);
+        if (QueryInst->mayReadOrWriteMemory())
+          return MemDepResult::getClobber(LI);
       }
 
       // FIXME: this is overly conservative.
       // While volatile access cannot be eliminated, they do not have to clobber
       // non-aliasing locations, as normal accesses can for example be reordered
       // with volatile accesses.
       if (LI->isVolatile())
         return MemDepResult::getClobber(LI);
if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
         if (SI->getOrdering() != Monotonic)
           return MemDepResult::getClobber(SI);
         if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst))
           if (!QueryLI->isSimple())
             return MemDepResult::getClobber(SI);
         if (auto *QuerySI = dyn_cast<StoreInst>(QueryInst))
           if (!QuerySI->isSimple())
             return MemDepResult::getClobber(SI);
+        if (QueryInst->mayReadOrWriteMemory())
+          return MemDepResult::getClobber(LI);
       }
 
       // FIXME: this is overly conservative.
       // While volatile access cannot be eliminated, they do not have to clobber
       // non-aliasing locations, as normal accesses can for example be reordered
       // with volatile accesses.
       if (SI->isVolatile())
         return MemDepResult::getClobber(SI);