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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1682
+        (PDom && isModOrRefSet(AA->getModRefInfo(PDom, SrcLoc))) ? PDom
+                                                                 : nullptr;
+    if (PDom && PDom->isTerminator() && PDomUser) {
----------------
khei4 wrote:
> nikic wrote:
> > khei4 wrote:
> > > 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.
> > Based on D157979, you can pass nullptr as the defining access and use createMemoryAccessInBB with `Begin` insertion place (when inserting at start of block).
> Sounds great! Thank you! I'll handle it.
Hm, I really don't like how we distinguish between pdom being a user and the start of a block here. I think it may be better if we explicitly distinguish these two cases during the analysis.

I had something like this in mind:
```
/// PDom is either an Instruction, or the start of a BasicBlock.
using PDom = PointerUnion<Instruction *, BasicBlock *>;
static PDom findNearestCommonPostDominator(PDom I1, PDom I2,
                                           PostDominatorTree *PDT) {
  auto GetParent = [](PDom I) {
    if (auto *BB = dyn_cast<BasicBlock *>(I))
      return BB;
    return cast<Instruction *>(I)->getParent();
  };
  BasicBlock *BB1 = GetParent(I1);
  BasicBlock *BB2 = GetParent(I2);
  if (BB1 == BB2) {
    if (isa<BasicBlock *>(I1))
      return I1;
    if (isa<BasicBlock *>(I2))
      return I2;
    return cast<Instruction *>(I1)->comesBefore(cast<Instruction *>(I2)) ? I2
                                                                         : I1;
  }
  BasicBlock *PDomBB = PDT->findNearestCommonDominator(BB1, BB2);
  if (!PDomBB)
    return nullptr;
  if (BB2 == PDomBB)
    return I2;
  if (BB1 == PDomBB)
    return I1;
  return PDomBB;
}
```
For `Instruction *` PDom you would use createMemoryAccesAfter, for `BasicBlock *` you would use `createMemoryAccessInBB`.


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

https://reviews.llvm.org/D155406