<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Sep 10, 2015 at 5:38 PM Pete Cooper via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><blockquote type="cite"><div>On Sep 10, 2015, at 5:30 PM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>> wrote:</div><br><div><div dir="ltr">I think '-load' is going to have to work *fundamentally* differently with the new pass manager, but I'm confident it will work there.</div></div></blockquote></div></div><div style="word-wrap:break-word"><div>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.</div></div></blockquote><div><br></div><div>While I'm glad, at the same time, don't stop looking for holes in my plan. That is much appreciated.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>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.</div></div></div></blockquote></div></div><div style="word-wrap:break-word"><div>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.</div><div><br></div><div>But, it would also make pass registration just like what we have now, which isn’t necessarily a good thing.</div></div></blockquote><div><br></div><div>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.</div><div><br></div><div>However, it would make the entire thing quite opaque IMO, and for pretty small gains. Some of the down sides here:</div><div><br></div><div>- 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.</div><div><br></div><div>- 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.</div><div><br></div><div>- 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.</div><div><br></div><div>- 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.</div><div><br></div><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>I was planning on building any plugin support at the very end, because it isn't an important use case to me. </div></div></div></blockquote></div></div><div style="word-wrap:break-word"><div>Makes sense.  I’d do the same if i was writing the new PM.</div></div><div style="word-wrap:break-word"><div><br><blockquote type="cite"><div><div dir="ltr"><div>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.</div></div></div></blockquote></div></div><div style="word-wrap:break-word"><div>I might try.  I honestly don’t use dynamically loaded passes either, or not often, but they are still useful to have.</div></div></blockquote><div><br></div><div>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.</div><div><br></div><div><br></div><div>Thanks again for the thoughts!</div><div>-Chandler</div></div></div>