[mlir][PassRegistry] Allow pipelines to be registered multiple times

[maxbartel]

Summary

Right now we only allow passes to be registered multiple times, as long
as they have the same TypeId. For PassPipelines this was simply not
allowed. This creates problems when you have a pipeline that is specific
to a dialect and load them both at the same time in the python bindings.
If you have a python file with multiple tests each having its own
context and want to load your dialect, you will hit this assert.

On a side-note: We are handling the dialect registry with the
MLIRContext. There is nothing similar for
passes, they are managed by a ManagedStatic StringMap. Is there a
specific reason for that?

[mehdi_amini]

I don't understand the use-case here, but that seems fishy and unsafe, I would question the setup that leads there instead.

[maxbartel]

In D158091#4624085, @mehdi_amini wrote:

I don't understand the use-case here, but that seems fishy and unsafe, I would question the setup that leads there instead.

It is quite easy to reproduce. When you have two python tests that should run independent from each other and you register the pass pipeline. Something like this:

def test1():
    register_custom_pass_pipeline()

def test2():
    register_custom_pass_pipeline()

This would work for passes, but will trigger the assert with pipelines.

To be clear, I don't think just allowing them is a nice solution, but at least it would then be aligned with the normal passes. Hence my question regarding the MLIRContext. I would also be happy to implement something else, but that would probably need an RFC. WDYT @mehdi_amini ?

[jpienaar]

This doesn't really provide you with the independence testing side, but rather hides that there is global state being mutated. Given tests can run in any order, Python side you probably need to do this registration at file scope.

[maxbartel]

What about letting MLIRContext also handle the pass registry? Is there something obvious going against that?

You are probably right about doing that at the file scope, but I kind of feel that this is also a workaround.

[mehdi_amini]

At some point, register_custom_pass_pipeline() will get into some C++, where you can make it idempotent by relying on a static variable.

The MLIRContext has to do with the IR entities, the pass management is fully decoupled: the IR library does not depend on the pass infra library.

[maxbartel]

Got it, thank you! I will close this then.

mlir/lib/Pass/PassRegistry.cpp

 // PassPipelineInfo
 //===----------------------------------------------------------------------===//
 
 void mlir::registerPassPipeline(
     StringRef arg, StringRef description, const PassRegistryFunction &function,
     std::function<void(function_ref<void(const PassOptions &)>)> optHandler) {
   PassPipelineInfo pipelineInfo(arg, description, function,
                                 std::move(optHandler));
-  bool inserted = passPipelineRegistry->try_emplace(arg, pipelineInfo).second;
-  assert(inserted && "Pass pipeline registered multiple times");
-  (void)inserted;
+  passPipelineRegistry->try_emplace(arg, pipelineInfo);
 }
 
 //===----------------------------------------------------------------------===//
 // PassInfo
 //===----------------------------------------------------------------------===//
 
 PassInfo::PassInfo(StringRef arg, StringRef description,
                    const PassAllocatorFunction &allocator)