[PATCH] D64179: [CodeGen] Define an interface for the new pass manager.

[Charles Zhang via Phabricator via llvm-commits]

czhang added a comment.

@fedor.sergeev

In D64179#1623993 <https://reviews.llvm.org/D64179#1623993>, @fedor.sergeev wrote:

> In D64179#1610998 <https://reviews.llvm.org/D64179#1610998>, @czhang wrote:
>
> > In D64179#1606205 <https://reviews.llvm.org/D64179#1606205>, @fedor.sergeev wrote:
> >
> > > I believe in order to make progress here we first need to settle down on a very principal matter -
> > >
> > >   what kind of IRUnit MachineFunction is?
> >
>
>
> Okey, everything you said there makes perfect sense. Thanks for clarification.
>
> >> There are many details to dive in, but first lets work on doing the basics right:
> >> 
> >>   MachineFunctionPassManager
> >>   FunctionToMachineFunctionPassAdaptor
> > 
> > This sentence doesn't quite parse for me. Could you please elaborate what you mean here? Which part of the basics specifically?
>
> I mean both should work :)
>  In particular, MachineFunctionPassManager.
>  It seems that you just defined a typedef in a hope that it will work... for one you need some tests to verify that.
>  And my guess is that as soon as you instantiate MachineFunctionPassManager::run you will hit missing interfaces that generic PassManager template refers to.
>  At least I'm sure you will hit PassInstrumentation ones.
>
> Lets first start with some basic unittests (see llvm/unittests/IR/PassManagerTest.cpp etc).


But everything does work :). The reason I know is because the 6 follow up patches after this allow llc to run and produce bitwise identical inputs for the 5 other passes I ported after this compared to the legacy pass manager. (using --run-new-pass after the prerequisite passes are manually run with stop-before). It's harder to see that's the case because I had split up this patch series,

Nonetheless, you are absolutely correct that some amount of unit testing is needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64179