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

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

This looks okay to me as a fairly conservative extension. Does requiring PDT have any impact on compile-time?



================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1568
+        else
+          return true;
+      }
----------------
Hm, this return true doesn't seem right to me if it the instruction is inside a loop. I think we still need to perform the reachability check in this case.


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1631
+    if (PDom) {
+      Builder.SetInsertPoint(PDom->getParent(), ++PDom->getIterator());
+      Builder.CreateLifetimeEnd(SrcAlloca, AllocaSize);
----------------
A possible issue here is that `PDom` could be a terminator, e.g. an `invoke`. In that case `++` will go past the end of the block. I think it would be fine to treat this case as if there is no PDom, as it's unlikely to occur in practice (generally, the pdom use will be in a lifetime intrinsic, which can't be a terminator).


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

https://reviews.llvm.org/D155406