[PATCH] D45535: [DSE] Teach the pass that atomic memory intrinsics are stores.

Daniel Neilson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 18 14:42:57 PDT 2018


dneilson planned changes to this revision.
dneilson added a comment.

In https://reviews.llvm.org/D45535#1071336, @dmgreen wrote:

> Am I right in saying these are treated in the same way as unordered atomic stores are? In that they can be deleted by following non-atomic stores, even if they are "stronger" atomically?  If so, this looks fine to me.


Yes, I believe that it correct -- at least that's the reasoning I've been going off of. The tests in test/Transforms/DeadStoreElimination/atomic.ll show unordered atomic stores essentially being treated the same as non-atomic stores, so if it's good there then it should be good here.



================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:868
       LoadedLoc = MemoryLocation::get(V);
     } else if (MemTransferInst *MTI = dyn_cast<MemTransferInst>(BBI)) {
       LoadedLoc = MemoryLocation::getForSource(MTI);
----------------
dmgreen wrote:
> This one? This function does it's own special version of DSE. It's about removing stores to dead stack objects iirc.
Good catch. Thanks!

Looks like there might not be a test for this case at all. I'll try to add one of those as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D45535





More information about the llvm-commits mailing list