[PATCH] Fix MemoryDependenceAnalysis in cases where QueryInstr is a CmpXchg or a AtomicRMW

Robin Morisset morisset at google.com
Thu Aug 21 18:25:54 PDT 2014


Hi jfb, reames,

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?

http://reviews.llvm.org/D5019

Files:
  lib/Analysis/MemoryDependenceAnalysis.cpp

Index: lib/Analysis/MemoryDependenceAnalysis.cpp
===================================================================
--- lib/Analysis/MemoryDependenceAnalysis.cpp
+++ lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -425,6 +425,8 @@
         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.
@@ -503,6 +505,8 @@
         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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D5019.12823.patch
Type: text/x-patch
Size: 851 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140822/dc460a56/attachment.bin>


More information about the llvm-commits mailing list