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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:868
       LoadedLoc = MemoryLocation::get(V);
     } else if (MemTransferInst *MTI = dyn_cast<MemTransferInst>(BBI)) {
       LoadedLoc = MemoryLocation::getForSource(MTI);
----------------
dneilson wrote:
> dneilson wrote:
> > 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.
> Made the change, but I'm having difficulty creating a test that gets to here. Calls to the memtransfer intrinsics end up getting into the conditional at line 838, above:
> 
> ```
>       // If the call might load from any of our allocas, then any store above
>       // the call is live.
>       DeadStackObjects.remove_if([&](Value *I) {
>         // See if the call site touches the value.
>         return isRefSet(AA->getModRefInfo(CS, I, getPointerSize(I, DL, *TLI)));
>       });
> ```
> 
> The source argument for the intrinsic aliases the stack object(s), and this bit of code at 868 ends up having no effect. 
Yes, I agree it's impossible to get here because of the condition on line 825; an AnyMemTransferInst is also a CallSite.  Please just delete this line, then.


Repository:
  rL LLVM

https://reviews.llvm.org/D45535