[PATCH] D72148: [DSE] Support traversing MemoryPhis.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

One general comment: this seems to do more than just lifting the MemoryPhi restriction. It's adding some cases that were were not added to the initial version, and that's great, but it would be good to add this in the patch description at least.
(e.g. continue upward search in DSE.cpp:1871, adding removal of partially overlapping stores)



================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1599
+      }
       MemoryUseOrDef *CurrentUD = dyn_cast<MemoryUseOrDef>(Current);
       if (!CurrentUD)
----------------
`cast` and remove null check below?


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1642
       if (isa<MemoryPhi>(UseAccess)) {
-        LLVM_DEBUG(dbgs() << "  ...  hit MemoryPhi\n");
-        return None;
+        if (KillingDef == UseAccess)
+          continue;
----------------
Can you clarify this? 
KillingDef is a MemoryDef, UseAccess is a MemoryPhi inside this if. How can they be equal?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72148