[PATCH] D104464: [LoopIdiom] Transform memmove-like loop into memmove (PR46179)

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1287
+    // Ensure that LoadBasePtr is after StoreBasePtr or before StoreBasePtr for
+    // negative stride.
+    int64_t LoadOff = 0, StoreOff = 0;
----------------
Finally, someone trying to add this transform figured out this check... I think this is the fourth patch trying to add memmove to loopidiom.

Do you need to compare the offset to the size of the load/store operations?  I think you might get some funny behavior if the operations overlap.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1340
     // have an alignment but non-atomic loads/stores may not.
     NewCall = Builder.CreateElementUnorderedAtomicMemCpy(
         StoreBasePtr, StoreAlign.getValue(), LoadBasePtr, LoadAlign.getValue(),
----------------
Do we need to make sure we don't fall into this codepath?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104464/new/

https://reviews.llvm.org/D104464