[PATCH] D159075: [MemCpyOpt] implement forward dataflow sensitive stack-move optimization

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1437
+           const DataLayout &DL) {
+  if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(UI)) {
+    // We treat a call to a lifetime intrinsic that covers the entire
----------------
Early return and reduce indent


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1447
+                cast<ConstantInt>(II->getArgOperand(0))->getSExtValue();
+            Size < 0 || uint64_t(Size) == AllocaSize)
+          return true;
----------------
I think these are supposed to just be unsigned, no < 0 check?


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1457
+        if (DL.getTypeStoreSize(SI->getValueOperand()->getType()) ==
+            (*AllocaSize).getFixedValue())
+          return true;
----------------
Don't need the .getFixedValue()?


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1671
 
-  if (!SrcAlloca->isStaticAlloca() || !DestAlloca->isStaticAlloca())
+  if (!SrcAlloca->isStaticAlloca() || !DestAlloca->isStaticAlloca()) {
+    LLVM_DEBUG(
----------------
This is redundant with getAllocationSize being nullopt


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

https://reviews.llvm.org/D159075