[PATCH] D60715: [ISEL] Collect argument's forwarding regs when lowering calls

[Nikola Prica via Phabricator via llvm-commits]

NikolaPrica added inline comments.


================
Comment at: lib/CodeGen/MIRPrinter.cpp:490
+  // Sort call info by position of call instructions.
+  std::sort(YMF.CallSitesInfo.begin(), YMF.CallSitesInfo.end(),
+            [](yaml::CallSiteInfo A, yaml::CallSiteInfo B) {
----------------
bjope wrote:
> From CodingStandards.rst:
> 
> >  As a rule of thumb, always make sure to use llvm::sort instead of std::sort.
> 
> 
Thanks for pointing that!


================
Comment at: lib/CodeGen/TargetInstrInfo.cpp:145
+    ++Tail;
+    MBB->erase(MI);
+  }
----------------
bjope wrote:
> NikolaPrica wrote:
> > aprantl wrote:
> > > I'd prefer writing two loops (or even better two std::algorithms) for updating and erasing and let the optimizer merge them.
> > I've tried with std::remove_if and llvm::remove_if but they require deleted MachineInstr copy assignment operator. So I came up with two solutions:
> > 
> > `Solution 1`
> > ```
> > // Update call site info and remove all the dead instructions
> >  // from the end of MBB.
> >  while (Tail != MBB->end()) {
> >    auto MI = Tail++;
> >    if (MI->isCall())
> >      MBB->getParent()->updateCallSiteInfo(&*MI);
> >    MBB->erase(MI);
> >  }
> > ```
> > 
> > Or 
> > 
> >  `Solution 2`
> > ```
> > // Update call site info.
> >  for (auto It = Tail; It != MBB->end(); ++It)
> >    if (It->isCall())
> >      MBB->getParent()->updateCallSiteInfo(&*It)
> > 
> >  // Remove all the dead instructions from the end of MBB.
> >  for (auto It = Tail; It != MBB->end();) {
> >    auto MI = It++;
> >    MBB->erase(MI);
> >  }
> > ```
> > 
> > @aprantl First example seams better than existing one. I'm wondering whether second with two loops is what you aimed to?
> The second loop in "solution 2" can be reduced to `MBB->erase(Tail, MBB->end());` (that is what it used to look like). No need to complicate that part ;-)
`MBB->erase(Tail, MBB->end());` recursively deletes elements. I taught that Adrian aimed to usage of two loops because optimizer will be able to merge them. Not sure whether it will be able to merge loop and recursive deletion call.


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

https://reviews.llvm.org/D60715