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

Tue Aug 15 06:30:29 PDT 2023

khei4 added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1626
+      auto *LastMA = MSSA->getMemoryAccess(LastUser);
+      auto *EndMA = MSSAU->createMemoryAccessAfter(End, LastMA, LastMA);
+      MSSAU->insertDef(cast<MemoryDef>(EndMA), /*RenameUses=*/true);
----------------
nikic wrote:
> khei4 wrote:
> > So, replace the second argument with LastMA->getDefiningAccess() works. It seems like getDefiningAccess is idempotent for MemoryDef.
> getDefiningAccess() is not idempotent for MemoryDef. However, it's still fine, because of something I didn't realize before: The defining access we specify here actually doesn't matter at all. It will always be replaced with the correct defining access in https://github.com/llvm/llvm-project/blob/98322d3eb43168a6a64c1a15a1e754e15c04aa2f/llvm/lib/Analysis/MemorySSAUpdater.cpp#L341.
> 
> I may look into refactoring these APIs, because the way they currently work is very confusing...
> getDefiningAccess() is not idempotent for MemoryDef

Hmm. Thanks! 

I see, `getPreviousDef` might be used on multi-BB cases; To reminder at least I can attach comments!

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

https://reviews.llvm.org/D153453