[PATCH] D15354: [InstCombine] Extend peephole DSE to handle unordered atomics

[JF Bastien via llvm-commits]

jfb added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:1089
@@ -1088,4 +1088,3 @@
     if (LoadInst *LI = dyn_cast<LoadInst>(BBI)) {
-      if (LI == Val && equivalentAddressValues(LI->getOperand(0), Ptr) &&
-          LI->isSimple())
+      if (LI == Val && equivalentAddressValues(LI->getOperand(0), Ptr))
         return EraseInstFromFunction(SI);
----------------
An ordered load has ordering semantics that you'd be losing. The *value* stored can be eliminated, but you still need to keep atomic *ordering*.

And you just can't remove `volatile` :-)

Though it looks like the write-back tests are already correct? I'm confused now.

================
Comment at: test/Transforms/InstCombine/store.ll:127
@@ +126,3 @@
+; same location, then the non-atomic one is undefined if we're actual
+; racing.  As such, we're free to pick either store under the assumption
+; that we're not racing with any other thread.
----------------
We should at least check that the latest value gets stored (here and below).


http://reviews.llvm.org/D15354