[PATCH] D18906: [DeadStoreElimination] Shorten beginning of memset overwritten by later stores

[Chad Rosier via llvm-commits]

mcrosier accepted this revision.
mcrosier added a comment.
This revision is now accepted and ready to land.

LGTM, assuming my minor nits are addressed.


================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:289
@@ -288,3 +288,3 @@
       case Intrinsic::memset:
       case Intrinsic::memcpy:
         // Do shorten memory intrinsics.
----------------
Any particular reason we don't support memmove?  Maybe add a FIXME if it's safe to transform memmove.

================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:303
@@ +302,3 @@
+static bool isShortenableAtTheBeginning(Instruction *I) {
+  // FIXME: Handle only memset for now. Supporting memcpy should be easily done
+  // by offsetting the source address.
----------------
Supporting memcpy/memmove should...

================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:430
@@ -421,1 +429,3 @@
+  // Another interesting case is if the later store overwrites the end of the
+  // earlier store
   //
----------------
Please add a period.

================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:658
@@ +657,3 @@
+                    ? InstWriteOffset - DepWriteOffset
+                    : DepLoc.Size - (InstWriteOffset - DepWriteOffset);
+
----------------
I agree with Jun on this one.


http://reviews.llvm.org/D18906