[llvm] r272595 - [PM/MergeLoadStoreMotion] Convert the logic to static functions.

[Daniel Berlin via llvm-commits]

Thanks so much. I hope you don't take it personally :)

In this case, i think it would have been *much* easier to simply remove the
pass related stuff from the mergedloadstoremotion class (Ie the inheritance
from functionpass, getanalysisusage, etc), create an object in the legacy
and new pass wrapper, and use it.


IE
PreservedAnalyses
MergedLoadStoreMotionPass::run(Function &F, AnalysisManager<Function> &AM) {
  auto *AA = &AM.getResult<AAManager>(F);
  auto *MD = AM.getCachedResult<MemoryDependenceAnalysis>(F);
  DominatorTree *DT = &AM.getResult<DominatorTreeAnalysis>(F);
  MemorySSA *MSSA = nullptr;
  if (UseMemorySSA) {
    MSSA = &AM.getResult<MemorySSAAnalysis>(F);
  }
  MergedLoadStoreMotion MLSM(AA, MD, MSSA, DT);

  if (!MLSM.runOnFunction(F))
    return PreservedAnalyses::all();
  return PreservedAnalyses::none();
}


and
  bool runOnFunction(Function &F) override {
    if (skipFunction(F))
      return false;

    AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
    auto *MDWP = getAnalysisIfAvailable<MemoryDependenceWrapperPass>();
    MD = MDWP ? &MDWP->getMemDep() : nullptr;
    DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
    MSSA = &getAnalysis<MemorySSAWrapperPass>().getMSSA();
    MergedLoadStoreMotion MLSM(AA, MD, MSSA, DT);
    return MLSM.runOnFunction(F);
  }

(for the legacy pass).


Since we are supposed to be getting rid of the legacy stuff eventually, one
would presume eventually the MLSM class will just become a pass class again
(who knows though :P)

Excuse the extra pieces, i'm actually almost all the way to making this
just work (albeit with the memoryssa merge on top of it)

This should be a trivial modification of your actual porting patch, but not
require this refactoring.




On Thu, Jun 16, 2016 at 10:09 AM, Davide Italiano <davide at freebsd.org>
wrote:

> On Thu, Jun 16, 2016 at 9:20 AM, Daniel Berlin <dberlin at dberlin.org>
> wrote:
> > (and note, you didn't explain, anywhere, why any of these changes are
> > necessary for the new PM, they look, at a glance, like completely
> orthogonal
> > refactorings)
> >
> > On Thu, Jun 16, 2016 at 9:07 AM, Daniel Berlin <dberlin at dberlin.org>
> wrote:
> >>
> >> I don't remember seeing this for review, and you've basically taken a
> very
> >> well organized class and completely destroyed its organizational
> structure
> >> by creating a mix of class member functions and random non-class static
> >> functions.
> >>
> >> I don't see this as an improvement, even for the new PM.
> >> I also don't think we should try to take every single pass that is a
> class
> >> in LLVM and necessarily turn it into a bunch of static functions that
> pass
> >> around what was previously class state.
> >>
> >> I'm going to ask you to revert and actually post a patch for review so
> >> this can be actually discussed.
> >>
>
> I'm reverting this now. Please let me know if there are other passes
> converted by me which you want to see reverted and I'll do it.
>
> Thanks,
>
> --
> Davide
>
> "There are no solved problems; there are only problems that are more
> or less solved" -- Henri Poincare
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160616/b4f97fa2/attachment.html>