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

Tue Aug 8 03:10:46 PDT 2023

khei4 added a comment.

I slightly modified the tests by https://github.com/llvm/llvm-project/commit/90ecb9d5b082e331a569f8c06f85289faa2a5c5f

- add `-verify-memoryssa` to the opt flag
- make memcpy defined on lifetime-missing test

================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1614
+    auto *StartMA = MSSAU->createMemoryAccessBefore(
+        Start, FirstMA->getDefiningAccess(), FirstMA);
+    MSSAU->insertDef(cast<MemoryDef>(StartMA), /*RenameUses=*/true);
----------------
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.

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

https://reviews.llvm.org/D153453