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

Tue Jul 18 02:24:35 PDT 2023

khei4 added a comment.

@nikic Thank you for the review! I'll handle the teminator problem on the single-BB first! I'll add tests and reopen it;)

================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1631
+    if (PDom) {
+      Builder.SetInsertPoint(PDom->getParent(), ++PDom->getIterator());
+      Builder.CreateLifetimeEnd(SrcAlloca, AllocaSize);
----------------
nikic wrote:
> nikic wrote:
> > 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).
> Heh, looks like exactly this issue got reported here: https://reviews.llvm.org/D153453#inline-1503870 I failed to realize that this can already be an issue with the initial patch, not just this extension...
Thank you! As you said exactly, I have forgotten to see it, implemented on pcwalton's patch.

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

https://reviews.llvm.org/D155406