Page MenuHomePhabricator

[NPM] Ability to add a pass before a previously registered one
Needs ReviewPublic

Authored by pratlucas on May 21 2021, 2:56 AM.

Details

Summary

This patch introduces the addPassBefore method to PassManager,
allowing a pass to be registered before another specific one that
may have been registered before it.

This is specially useful for pass plugins, which do not go through the
Pass Registry and lack some of the granularity for controlling when
they should be run.

Diff Detail

Unit TestsFailed

TimeTest
30 msx64 debian > LLVM-Unit.IR/_/IRTests::PassManagerTest.AddPassBefore
Note: Google Test filter = PassManagerTest.AddPassBefore [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up.
30 msx64 windows > LLVM-Unit.IR/_/IRTests_exe::PassManagerTest.AddPassBefore
Note: Google Test filter = PassManagerTest.AddPassBefore [==========] Running 1 test from 1 test suite.

Event Timeline

pratlucas created this revision.May 21 2021, 2:56 AM
pratlucas requested review of this revision.May 21 2021, 2:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2021, 2:56 AM
aeubanks edited reviewers, added: asbirlea; removed: chandlerc.May 21 2021, 9:52 AM

I'd rather not have something like this, especially something that depends on pass names.
What exactly are you trying to do with this?

Hi @aeubanks,
As I mentioned, this is mostly targeted on plugin passes. At the moment the NPM only offers the PassBuilder's callbacks level of granularity to control when a plugin pass should be run, which might not be sufficient on some cases. The goal was to provide a more fine grained control over this, allowing plugin implementations to run before other native or plugin passes that migth impact their behavior.
Do you have any suggestions on alternative ways to achieve this? I'm not fond of using pass names here myself and would be more than happy to go for a different implementation.

Hi @aeubanks,
As I mentioned, this is mostly targeted on plugin passes. At the moment the NPM only offers the PassBuilder's callbacks level of granularity to control when a plugin pass should be run, which might not be sufficient on some cases. The goal was to provide a more fine grained control over this, allowing plugin implementations to run before other native or plugin passes that migth impact their behavior.
Do you have any suggestions on alternative ways to achieve this? I'm not fond of using pass names here myself and would be more than happy to go for a different implementation.

I'd still like to understand exactly what you'd be using this for. As in why does some pass need to run before all other instances of some other pass?

I'd still like to understand exactly what you'd be using this for. As in why does some pass need to run before all other instances of some other pass?

The concrete use case I have is based on two separate plugin passes - one of them is responsible for inserting a few inline assembly instructions into functions according to a specific criteria and the second one's job is to make some adjustments to all the inline asm instructions in the module. The former needs to run before the latter to ensure all inline asm instructions are in place before the adjustments take place.
Both of those plugin passes are required to run on the last step of the optimizer pipeline, using the registerOptimizerLastEPCallback extension point, so there's not enough granularity available to ensure those are run in this specific order.

I'd still like to understand exactly what you'd be using this for. As in why does some pass need to run before all other instances of some other pass?

The concrete use case I have is based on two separate plugin passes - one of them is responsible for inserting a few inline assembly instructions into functions according to a specific criteria and the second one's job is to make some adjustments to all the inline asm instructions in the module. The former needs to run before the latter to ensure all inline asm instructions are in place before the adjustments take place.
Both of those plugin passes are required to run on the last step of the optimizer pipeline, using the registerOptimizerLastEPCallback extension point, so there's not enough granularity available to ensure those are run in this specific order.

If you add both in the same callback passed to registerOptimizerLastEPCallback() you can control the order they're added, e.g.

PB.registerOptimizerLastEPCallback(
    [this](ModulePassManager &PM, PassBuilder::OptimizationLevel Level) {
      FunctionPassManager FPM;
      FPM.addPass(BarPass());
      FPM.addPass(FooPass());
      PM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
    });

This solution works well if both passes are always supposed to be run together. If they should also work as standalone passes, it becomes tricky to figure out at what point each of them should be registered.
This could be done using the legacy pass manager on a hacky kind of way by querying for the passes on the PassRegistry, but this is no longer an option in NPM.

The concrete use case I have is based on two separate plugin passes - one of them is responsible for inserting a few inline assembly instructions into functions according to a specific criteria and the second one's job is to make some adjustments to all the inline asm instructions in the module. The former needs to run before the latter to ensure all inline asm instructions are in place before the adjustments take place.
Both of those plugin passes are required to run on the last step of the optimizer pipeline, using the registerOptimizerLastEPCallback extension point, so there's not enough granularity available to ensure those are run in this specific order.

I'm still not understanding exactly what the criteria you're looking for is. Could you give a specific example of how your example could go wrong using registerOptimizerLastEPCallback? Even if they are to be registered separately via multiple calls to registerOptimizerLastEPCallback, the first registerOptimizerLastEPCallback should happen first.