[PATCH] D28909: [InstCombineCalls] Unfold element atomic memcpy instruction

[Igor Laevsky via Phabricator via llvm-commits]

igor-laevsky marked 7 inline comments as done.
igor-laevsky added a comment.

Second part: https://reviews.llvm.org/rL294453



================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:166
+    // We know alignment of the first element. It is also guaranteed by the
+    // verifier that element size is less or equal than first element alignment
+    // and both of this values are powers of two.
----------------
sanjoy wrote:
> I'd also prefer adding an assert here that checks what you said in the comment.
Proper checks of this invariants are quite verbose and will clobber the actual logic. I would prefer to rely on the verifier in this case.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:182
+
+  // Set the number of elements of the copy to 0, it will be deleted on the
+  // next iteration.
----------------
sanjoy wrote:
> Is it difficult to delete `AMI` right here?  If not, I'd just do that -- no point in waiting for another iteration if it is obvious what will happen.
Philip suggested that I should stick to the same process as other memory intrinsic optimizations do.  I don't have a strong opinion here, but I would go with Philip's suggestion since I already implemented it.


Repository:
  rL LLVM

https://reviews.llvm.org/D28909