[llvm-commits] [llvm] r131581 - in /llvm/trunk: include/llvm/DefaultPasses.h include/llvm/Support/StandardPasses.h lib/Support/StandardPasses.cpp

[David Chisnall]

On 21 May 2011, at 19:21, Chris Lattner wrote:

>> On 21 May 2011, at 18:56, Chris Lattner wrote:
>> Hi David,
>> The concrete use case that I have is a set of optimisations that are specific to the GNU Objective-C runtimes, and want to be run in the middle of the standard set of optimisations.  LLVM lacked a way of doing this without making invasive changes to the front ends that might use them (Clang, DragonEgg and LanguageKit in this case).  Now, people can relatively easily use them[1].  
> 
> Ok, I think that's a good goal.  I guess I just don't like the implementation approach to getting to it.
> 
>> One of the design goals of LLVM was to be modular and easily extensible.  This makes it easy for someone to extend (and, with some small changes that I plan on doing before 3.0) completely replace the set of optimisations that is run.  
>> 
>> I'd like to add a config file to LLVM that automatically loads installed plugins, so people can write new passes as plugins, and have them automatically used.
>> 
>> This is also useful when testing optimisations.  I have a release build of LLVM and Clang, but I can trivially write a new optimisation, tell clang (or some other front end) to use it, and check that it works and see what the performance change is, without needing to recompile anything other than the plugin.  Taking this further, it lets people test different default optimisation sets and orderings without a recompile, which makes it easier to pick a good set, because different options can be tried more easily.
> 
> I don't like the patch for several reasons:
> 1. Tons of macros.

There aren't many.  As I recall, there are none that are actually exposed, there are just 4 for avoiding copy-and-paste coding in StandardPasses.h

> 2. Exposing tons of PassID's.

True.  In fact, this doesn't expose as many as are really required, because passes that appear multiple times in the sequence should have unique IDs, to allow just one to be replaced.  

> 3. You added a bunch of static constructors for the static globals in the new .cpp file.

I'm not sure where you're talking about here.  Can you be more specific?

> Of these, the third is the easiest to solve.  However, can we take a step back and see what problem you'd trying to solve?
> 
> While it's theoretically possible to want to control every possible aspect of the standard pipeline from a plugin, I don't think that it is realistically needed.  Instead, I think there are a few different classes of optimization pass, for example:
> 
> 1. Something that wants to run at the very start, or very end of the pipeline. (can be any pass)
> 2. Something that runs after the early per-function passes. (must be a functionpass)
> 3. Something that runs early in the CGSCC passmgr.  (must be a cgsccpass or functionpass)
> 4. Something that runs late in the functionpass pipeline (must be a functionpass)
> 
> I don't know where you pass fits in, but I'd guess it is #4.

Some of mine are ModulePasses (e.g. caching the class lookup - a function pass wouldn't be able to share the caches globally).  

The current code doesn't yet allow replacing existing passes, but it's something that I'd like to add.  For example, I can easily imagine that someone would write a better alias analysis pass, or a pass that had some functionality that overlapped with one of the default set and wanted to replace it.

> Instead of your current approach, how about we introduce a new "OptimizerBuilder" class. The API would be something like:
> 
> OptimizerBuilder Builder;
> Builder.setOptimizationLevel(2);
> Builder.addEarlyCGSCCPass(new mypass());
> Builder.addLastFunctionPass(new myotherpass());
> Builder.populatePassManager(PM);
> 
> I think that something like this would give us the extensibility that you need, and would clean up the previously existing nasty API for setting up the pass manager.  What do you think?  

I'm not sure how this would be used.  Can you give some concrete examples of how, with this scheme, a plugin would:

- Register a new pass to run at optimisation level 2
- Register a new pass to run at any optimisation level, except when we are optimising for size
- Replace an existing pass with a different implementation
- Register a new pass to run at optimisation level 2, before another pass from another plugin, and only if that other plugin is actually loaded

> Are you ok with reverting the patch and trying a different approach?

I'd rather not revert it, since I'm using this functionality, but I've no problem with replacing it with something better - as far as I'm concerned the design is not finalised until 3.0 ships, and I'm happy to make changes to it, including significant architectural changes, as long as we end up with something useable at the end.  

David