[PATCH] D12773: [PM] Port SROA to the new pass manager.

[Chandler Carruth via llvm-commits]

On Thu, Sep 10, 2015 at 5:38 PM Pete Cooper via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> On Sep 10, 2015, at 5:30 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
>
> I think '-load' is going to have to work *fundamentally* differently with
> the new pass manager, but I'm confident it will work there.
>
> Oh yeah, I’m not surprised it’ll be different from the current code.
> Given that you know what you’re writing much more than the rest of us, I
> trust you when you say it will work.
>

While I'm glad, at the same time, don't stop looking for holes in my plan.
That is much appreciated.


>
> The idea is that when you load a pass as a plugin, you get a callback that
> allows you to add your own passes to the pass manager. The pass manager is
> already doing full type erasure so it can have passes in it which it has
> never seen before.
>
> So I hate to write this, but this sounds very similar to the current pass
> initializer scheme, just that you are using a callback instead of a static
> initializer.  Why have the difference?  I mean why not use the callback for
> all passes? Then the code is the same regardless of dynamic loading, and it
> seems to me like that would reduce the amount of code you had to put in
> SROA.h for example.
>
> But, it would also make pass registration just like what we have now,
> which isn’t necessarily a good thing.
>

There is definitely some merit to this general idea. We could go with a
system where each .cc file essentially exposed a very narrow interface to
the rest of the system. We could have each each pass register a callback to
handle parsing and construction of the pass and injection into the manager,
etc.

However, it would make the entire thing quite opaque IMO, and for pretty
small gains. Some of the down sides here:

- It would become circuitous how to create a unittest to check a pass under
very *particular* circumstances. While this is rare in LLVM (we can usually
just use file-check tests), we keep wanting to do it every now and
something I rather like about the new system is that this becomes, if
anything, quite a bit easier.

- We would add a lot of layers of indirection for clients that want to do
things like low-latency JITs. A nice aspect of having a more "normal" C++
interface is that things like inlining work again, and there are relatively
few indirection boundaries such as we have in the current pass manager.
While it *shouldn't* matter, I'm a bit worried that the current pass
manager has gotten so bad that it actually causes performance problems for
folks.

- Its an unnatural requirement -- it is a layer of indirection that does
not serve some *other* abstraction purpose. This to me is probably the
strongest reason. It ends up with a strong code smell for me.

- The killer for me: it simply doesn't work for analyses, where we *need*
API-level access. And this is why today, analyses all already do this, and
it is just transforms that have a special pattern. Personally, I'd rather
that all of these use the same pattern. I find it much cleaner for
plugin-passes to be the odd ones out, than for all the transforms to be
different in their core approach from the analyses.


But both approaches do clearly work, and this is a tradeoff at a very high
level. While I've been charging the other direction, we could pull back and
make everything below the analyses look more like a hermetically sealed
plugin. But I'm currently still leaning the same direction, so I'd want to
understand why we should pull back and change course.


>
> I was planning on building any plugin support at the very end, because it
> isn't an important use case to me.
>
> Makes sense.  I’d do the same if i was writing the new PM.
>
> It'd be great if people that are actually using this could contribute it,
> as otherwise I'm much more likely to focus on essentially all the other use
> cases until it comes time to try to rip *out* the old pass manager.
>
> I might try.  I honestly don’t use dynamically loaded passes either, or
> not often, but they are still useful to have.
>

They are definitely useful to have. The fact that they'll be last for me is
just because other stuff is much more urgent. It'll be nice to have in
place either way because it will help keep the design really clearly pinned
down.


Thanks again for the thoughts!
-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150911/111c94f7/attachment.html>