[PATCH] D87414: [DSE] Make sure that DSE+MSSA can handle masked stores

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:412
 
+/// Check if two instruction are masked stores that completely
+/// overwrite one another.
----------------
Might be good to state that Later must overwrite Earlier. We could check that the mask of `Later` is a superset of the mask of `Earlier`. Might be good to add a TODO.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:424
+  // Pointers.
+  if (IIL->getArgOperand(1) != IIE->getArgOperand(1))
+    return OW_Unknown;
----------------
Wouldn't it be better to check if the pointers are must-alias?


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1722
   /// Returns true if \p Use completely overwrites \p DefLoc.
-  bool isCompleteOverwrite(MemoryLocation DefLoc, Instruction *UseInst) {
+  bool isCompleteOverwrite(MemoryLocation DefLoc, Instruction *DefInst,
+                           Instruction *UseInst) {
----------------
Could you update the doc-comment?


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1835
+  bool mayReadFromMemory(IntrinsicInst *II) {
+    switch (II->getIntrinsicID()) {
+    case Intrinsic::masked_store:
----------------
Is it possible to mark `masked_store` as `IntrWriteMem`? Then `mayReadFromMemory` should automatically return false for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87414