[PATCH] D97667: [loop-idiom] Hoist loop memcpys to loop preheader

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

To clarify, i was essentially asking to get an independent reviewer to re-review this,
looks like that didn't happen.



================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:856
+  // that every byte is touched in the loop.
+  const SCEVConstant *StrStride =
+      dyn_cast<SCEVConstant>(StoreEv->getOperand(1));
----------------
This is the first time i'm seeing `Str` short for `Store`. Please use it direcly.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:868-870
+  // Check if the load stride matches the store stride.
+  if (StrIntStride != LoadIntStride && StrIntStride != -LoadIntStride)
+    return false;
----------------
This doesn't make sense. Strides of load and store must match exactly.
Doesn't this miscompile the case where load goes forward and store backward or vice verse?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97667