[PATCH] D155406: [MemCpyOpt] implement multi BB stack-move optimization

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1434-1437
+    if (isa<BasicBlock *>(I1))
+      return I1;
+    if (isa<BasicBlock *>(I2))
+      return I2;
----------------
I think I got these the wrong way around.

If `I1` is the start of the block, then `I2` is either also the start of the block or comes later in it, so it post-dominates.


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1571
 
+  // TODO: update comment
   // 3. Check that dest has no Mod/Ref, except full size lifetime intrinsics,
----------------
Remove TODO?


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1681
+      if (auto *PDomI = dyn_cast<Instruction *>(PDom)) {
+        // If PDom is Instruction ptr, insert after bcecause it's user of
+        // SrcAlloca.
----------------
because


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1687
+                                               MSSA->getMemoryAccess(PDomI));
+      } else if (auto *PDomB = dyn_cast<BasicBlock *>(PDom)) {
+        Builder.SetInsertPoint(PDomB, PDomB->begin());
----------------
And drop the else below.


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

https://reviews.llvm.org/D155406