[PATCH] D97676: [DSE] Extending isOverwrite to support offsetted fully overlapping stores

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1314
   const DataLayout &DL = BB.getModule()->getDataLayout();
+  BatchAAResults BatchAA(*AA, /*CacheOffsets =*/true);
   bool MadeChange = false;
----------------
fvrmatteo wrote:
> nikic wrote:
> > Is BatchAA safe to use in the non-MSSA DSE?
> I'm not sure it's safe because on the MD-based DSE I see that the IR is modified and the BatchAAResults should be used when there are no IR changes in between queries. At the same time plain AliasAnalysis is not providing the offset information, so I guess I should be dropping BatchAA here for now, even though the isOverwrite will try to call `getClobberOffset` on the AA reference.
Some types of IR modifications (removing instructions if no new ones are added) can be okay, and the MSSA-based implementation uses that. I'm just not familiar with the old one. Maybe we can avoid the question altogether though, by just deleting this code? D97877


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

https://reviews.llvm.org/D97676