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

[Kohei Asano via Phabricator via llvm-commits]

khei4 added a comment.

@vitalybuka @bgraur @joanahalili
Thank you for reporting this!

FWIW, this transformation itself ensures 

1. allocas are never captured on IR level. (This is the requirements for this transformation.)
2. newly inserted lifetime.start dominates all use of memory location, and inserted end post-dominates those.

So, at least on the IR level, I believe this doesn't introduce any problematic use for the memory location, but certainly, this changes the memory use of the original source code, and it may introduce the unexpected use of the address, out of the scope that original source code had, by inserting (post)dominating lifetime intrinsics. I assumed lifetime markers are just overwriting the memory location by undef values, and the original lifetime ranges (scope?) for the memory location could be changed by the optimization if we correctly attach lifetime intrinsics, but I think it was wrong for the asan, right? (I'm not sure about the actual false positive case, any concrete counter-example will be appreciated.)
I should have cared about the original lifetime to kindly introduce this optimization, as vitaly says https://reviews.llvm.org/D153453#4556386. I believe this optimization is effective practically e.g. for the Rust programs that introduce memcpy for move, so splitting the patch sounds reasonable.

Assuming that inserting the new lifetime marker(or use) for an address out of the original scope would be problematic for asan, I want to try to use the single range lifetime completely covered other one, as vitaly suggested, for the first step!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155406