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

[Dawid Jurczak via Phabricator via llvm-commits]

yurai007 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;
----------------
efriedma wrote:
> 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.
> Finally, someone trying to add this transform figured out this check... I think this is the fourth patch trying to add memmove to loopidiom.
Well, hopefully there won't be fifth one :)

> 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.
Yes, it would be nice to prevent overlapping. I'm going to add protection against it and UT reproducer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104464