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

[Kohei Asano via Phabricator via llvm-commits]

khei4 marked an inline comment as done.
khei4 added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1598
+        if (LI && LI->getLoopFor(BB) != nullptr)
+          return false;
+
----------------
nikic wrote:
> Is this check needed? I would expect that this is covered by adding the successors to the reachability worklist. Then we shouldn't need LoopInfo.
Hmm, you seem right. At least the current test results don't change. I could erase the LoopInfo!


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1682
+        (PDom && isModOrRefSet(AA->getModRefInfo(PDom, SrcLoc))) ? PDom
+                                                                 : nullptr;
+    if (PDom && PDom->isTerminator() && PDomUser) {
----------------
nikic wrote:
> I don't get what you're doing here with AA. Why is this necessary now and wasn't needed in the single BB case?
This was to know whether the post dominator has MemoryAccess on SrcAlloca.
If it has MemoryAccess, check whether it's terminator or not, and find the place to insert MemoryAccess.
If it's not, currently skipped because I can't know how to insert MemoryDef or MemoryPhi consistently with any post dominators.


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

https://reviews.llvm.org/D155406