[PATCH] D30225: [LIR] re-enable generation of memmove with runtime checks

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:508
   // Otherwise, see if the store can be turned into a memcpy.
-  if (HasMemcpy) {
+  if (HasMemcpy || HasMemmove) {
     // Check to see if the stride matches the size of the store.  If so, then we
----------------
This would not work right for if the target has memmove, but not memcpy.  Maybe not an issue in practice, but still kind of confusing.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:986
+    }
+  }
+
----------------
This whole worklist thing looks very suspicious; for the purpose of figuring out whether the memmove covers the loop, why do you need to special-case the pointer operand of the load instruction?


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1240
+    IRBuilder<> CondBuilder(MemmoveB);
+    CondBuilder.CreateBr(ExitB);
+    CondBuilder.SetInsertPoint(MemmoveB->getTerminator());
----------------
This is breaking LoopSimplify form for the loop.


https://reviews.llvm.org/D30225