[PATCH] D112313: [DSE] Optimize defining access of defs while walking upwards.

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1486
+              (OR == OW_Complete || OR == OW_MaybePartial))
+            KillingDef->setOptimized(CurrentDef);
+
----------------
fhahn wrote:
> nikic wrote:
> > I think this is incorrect if KillingDef accesses multiple locations. Say you have a memcpy, then KillingLoc will point to the write location only, and that's the only one for which we will check aliasing. This means that the optimized access might look past a clobber of the read location.
> But isn't the optimized access only for write clobbers? This is the part that is not really documented well, but going from the use in `getClobberingMemoryAccess` it looks like it should be the nearest dominating write clobber
Yes, it's about write clobbers. The problem is that a MemoryDef may access multiple locations, and the optimized access is the nearest write clobber on any of those locations. Your code (unless I'm missing something) will only find clobbers to the write location of KillingDef, but not to any additional read locations it may have.

```
; assuming p, q noalias
write(p) <-- optimized access found by this implementation
write(q) <-- correct optimized access
memcpy(p, q)
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112313/new/

https://reviews.llvm.org/D112313