[PATCH] D42889: [LoopIdiomRecognize] Add support for memmove. Fixes PR25165

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Some quick comments on the code etc., want to think a bit harder about the loop access checks just to make sure I'm not missing anything. Ben may also have thoughts there.



================
Comment at: lib/IR/IRBuilder.cpp:169-186
+  if (DstAlign > 0)
+    MCI->setDestAlignment(DstAlign);
+  if (SrcAlign > 0)
+    MCI->setSourceAlignment(SrcAlign);
+
+  // Set the TBAA info if present.
+  if (TBAATag)
----------------
Maybe factor this out and share it between MemCpy and MemMove?


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:484
+    if (UnorderedAtomic || LI->isAtomic())
+      return HasMemcpy ? LegalStoreKind::UnorderedAtomicMemcpy
+                       : LegalStoreKind::None;
----------------
Why the check for HasMemcpy here? Seems a bit odd to check it here rather than later....


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1056-1057
+  bool IsAtomicLoadOrStore = SI->isAtomic() || LI->isAtomic();
+  assert((!IsAtomicLoadOrStore || !PerformMemmove) &&
+         "cannot memmove atomic load or store");
+
----------------
Maybe leave a FIXME that we should build an atomic memmove lowering much like we did for memcpy that has specified element-width operations....


https://reviews.llvm.org/D42889