[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:467
+  // Query the alias information
+  AliasResult AAR = AA.alias(Later, Earlier);
+
----------------
Why not continue using P1 and P2 here? Of course AA will also strip pointer casts, but if we already did it, we may as well pass the stripped pointers.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:471
   // the later store was larger than the earlier store.
-  if (P1 == P2 || AA.isMustAlias(P1, P2)) {
+  if (P1 == P2 || (AAR == AliasResult::MustAlias)) {
     // Make sure that the Later size is >= the Earlier size.
----------------
As you are performing the alias query unconditionally now, the separate `P1 == P2` shortcut here isn't useful anymore, you can drop it. (I also checked whether your change may have a compile-time impact due to the unconditional query, but it does not seem so.)


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1314
   const DataLayout &DL = BB.getModule()->getDataLayout();
+  BatchAAResults BatchAA(*AA, /*CacheOffsets =*/true);
   bool MadeChange = false;
----------------
Is BatchAA safe to use in the non-MSSA DSE?


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

https://reviews.llvm.org/D97676