[PATCH] D153453: [MemCpyOpt] implement single BB stack-move optimization which unify the static unescaped allocas

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1614
+    auto *StartMA = MSSAU->createMemoryAccessBefore(
+        Start, FirstMA->getDefiningAccess(), FirstMA);
+    MSSAU->insertDef(cast<MemoryDef>(StartMA), /*RenameUses=*/true);
----------------
khei4 wrote:
> nikic wrote:
> > Why does this use getDefiningAccess() now? I would expect the call to be just `MSSAU->createMemoryAcessBefore(Start, FirstMA)`.
> Sorry, I didn't get the point and just mimicked [[ https://github.com/llvm/llvm-project/blob/90ecb9d5b082e331a569f8c06f85289faa2a5c5f/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp#L1297-L1300 | the other use on MemCpyOpt ]].
> 
> > You should omit the third parameter (Template) here, because the effects of the lifetime intrinsics have no relation to those of FirstMA.
> Do you mean `createDefinedAccess`? 
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/MemorySSA.cpp#L1682-L1685
> The third argument for createMemoryAccessBefore seems necessary(I'm not sure whether this is inherently so) https://github.com/llvm/llvm-project/blob/71c3a5519dbcd609fb64560ac7fdfe8db149b905/llvm/include/llvm/Analysis/MemorySSAUpdater.h
> 
> 
> 
> > Also, shouldn't we be inserting the access *before* the first user, rather than after?
> 
> This seems exactly right! Applied.
Ah, ignore my comment, you are right.


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

https://reviews.llvm.org/D153453