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

[Kohei Asano via Phabricator via llvm-commits]

khei4 added a comment.

@nikic 
Thank you for the review!



================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1650
+    while (PDom && PDom->isTerminator()) {
+      auto *PostDomBB = (*PDT)[PDom->getParent()]->getIDom()->getBlock();
+      PDom = PostDomBB ? PostDomBB->getFirstNonPHI() : nullptr;
----------------
nikic wrote:
> This looks suspicious to me. Would getBlock() return nullptr, or already getIDom()?
Hmm, certainly. So far, I'm also not sure about this behavior, for the current invoke example.
when `PDom = %rv = invoke i32 @use_nocapture(ptr %src) to label %suc unwind label %unw`
```
((*PDT)[PDom->getParent()]->getIDom() == nullptr) => 0
((*PDT)[PDom->getParent()]->getIDom()->getBlock() == nullptr) => 1
```
Something wrong might be on PDom. Anyway, I could handle both `nullptr` conservatively.


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1568
+        else
+          return true;
+      }
----------------
nikic wrote:
> Hm, this return true doesn't seem right to me if it the instruction is inside a loop. I think we still need to perform the reachability check in this case.
Right! We need to use the batch BB reachability for instructions!


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

https://reviews.llvm.org/D155406