[PATCH] D40480: MemorySSA backed Dead Store Elimination.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

Does this support any transforms which aren't supported by the memdep-based DSE?



================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:1425
+
+// FIXME: Make this use MSSA?
+bool eliminateNoopStore(MemoryDef &MD, InstOverlapIntervalsTy &IOL,
----------------
It should be very easy to implement memoryIsNotModifiedBetween using MSSA, I think?  But not a big deal to leave it for now.


================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:1469
+
+bool isThrowInRange(Value *SI, Value *NI, const Value *SILocUnd,
+                    DenseMap<Value *, unsigned> &InstructionPostOrders,
----------------
This is a confusing name, given what the function checks.


================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:1651
+        if (MayThrow)
+          MayThrowsPostOrders.push_back(IPO);
+        if (MD && hasMemoryWrite(&I, TLI) && isRemovable(&I))
----------------
Is it possible for any DSE-related transform to invalidate this?

I'm not sure I really understand how you're using MayThrowsPostOrders here... more comments would be helpful.


================
Comment at: test/Transforms/DeadStoreElimination/simple.ll:555
+; I think this case is currently handled incorrectly by memdeps dse
+; throwing should leave store i32 1, not remove from the free.
+declare void @free(i8* nocapture)
----------------
Yes, you're right; I guess I missed that case when I fixed the other noalias issues in DSE.


https://reviews.llvm.org/D40480