shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[MLIR] Make PassPipelineOptions virtually inheriting from PassOptions to allow diamond inheritance (#146370)

Browse Source 
## Problem

Given 3 pipelines, A, B, and a superset pipeline AB that runs both the A
& B pipelines, it is not easy to manage their options - one needs to
manually recreate all options from A and B into AB, and maintain them.
This is tedious.

## Proposed solution
Ideally, AB options class inherits from both A and B options, making the
maintenance effortless. Today though, this causes problems as their base
classes `PassPipelineOptions<A>` and `PassPipelineOptions<B>` both
inherit from `mlir::detail::PassOptions`, leading to the so called
"diamond inheritance problem", i.e. multiple definitions of the same
symbol, in this case parseFromString that is defined in
mlir::detail::PassOptions.

Visually, the inheritance looks like this:

```
                         mlir::detail::PassOptions
                            ↑                  ↑
                            |                  |
           PassPipelineOptions<A>      PassPipelineOptions<B>
                            ↑                  ↑
                            |                  |
                         AOptions           BOptions
                            ↑                  ↑
                            +---------+--------+
                                      |
                                  ABOptions
```

A proposed fix is to use the common solution to the diamond inheritance
problem - virtual inheritance.
This commit is contained in:
Sasa Vuckovic 2025-08-08 12:33:56 +02:00 committed by GitHub
parent bd39ae6125
commit 9349484e8f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 70 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
mlir/include/mlir/Pass/PassOptions.h
View File

@ -377,7 +377,7 @@ private:
/// ListOption<int> someListFlag{*this, "flag-name", llvm::cl::desc("...")};
/// };
template <typename T>
class PassPipelineOptions : public detail::PassOptions {
class PassPipelineOptions : public virtual detail::PassOptions {
public:
/// Factory that parses the provided options and returns a unique_ptr to the
/// struct.

10
mlir/test/Pass/pipeline-options-parsing.mlir
View File

@ -13,6 +13,7 @@
// RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.module(builtin.module(func.func(test-options-pass{list=3}), func.func(test-options-pass{enum=one list=1,2,3,4 string=foo"bar"baz})))' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_6 %s
// RUN: mlir-opt %s -verify-each=false '-test-options-super-pass-pipeline=super-list={{enum=zero list=1 string=foo},{enum=one list=2 string="bar"},{enum=two list=3 string={baz}}}' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_7 %s
// RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.module(func.func(test-options-super-pass{list={{enum=zero list={1} string=foo },{enum=one list={2} string=bar },{enum=two list={3} string=baz }}}))' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_7 %s
// RUN: mlir-opt %s -verify-each=false -test-options-super-set-ab-pipeline='foo=true bar=false' -dump-pass-pipeline 2>&1 | FileCheck --check-prefix=CHECK_11 %s
// This test checks that lists-of-nested-options like 'option1={...},{....}' can be parsed
@ -106,3 +107,12 @@
// CHECK_10-NEXT: test-options-pass{enum=zero string= string-list={,}}
// CHECK_10-NEXT: )
// CHECK_10-NEXT: )
// CHECK_11: builtin.module(
// CHECK_11-NEXT: func.func(
// CHECK_11-NEXT: test-options-pass-a
// CHECK_11-NEXT: )
// CHECK_11-NEXT: func.func(
// CHECK_11-NEXT: test-options-pass-b
// CHECK_11-NEXT: )
// CHECK_11-NEXT: )

59
mlir/test/lib/Pass/TestPassManager.cpp
View File

@ -133,6 +133,51 @@ struct TestOptionsSuperPass
llvm::cl::desc("Example list of PassPipelineOptions option")};
};
struct TestOptionsPassA
: public PassWrapper<TestOptionsPassA, OperationPass<func::FuncOp>> {
MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestOptionsPassA)
struct Options : public PassPipelineOptions<Options> {
Option<bool> foo{*this, "foo", llvm::cl::desc("Example boolean option")};
};
TestOptionsPassA() = default;
TestOptionsPassA(const TestOptionsPassA &) : PassWrapper() {}
TestOptionsPassA(const Options &options) { this->options.foo = options.foo; }
void runOnOperation() final {}
StringRef getArgument() const final { return "test-options-pass-a"; }
StringRef getDescription() const final {
return "Test superset options parsing capabilities - subset A";
}
Options options;
};
struct TestOptionsPassB
: public PassWrapper<TestOptionsPassB, OperationPass<func::FuncOp>> {
MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestOptionsPassB)
struct Options : public PassPipelineOptions<Options> {
Option<bool> bar{*this, "bar", llvm::cl::desc("Example boolean option")};
};
TestOptionsPassB() = default;
TestOptionsPassB(const TestOptionsPassB &) : PassWrapper() {}
TestOptionsPassB(const Options &options) { this->options.bar = options.bar; }
void runOnOperation() final {}
StringRef getArgument() const final { return "test-options-pass-b"; }
StringRef getDescription() const final {
return "Test superset options parsing capabilities - subset B";
}
Options options;
};
struct TestPipelineOptionsSuperSetAB : TestOptionsPassA::Options,
TestOptionsPassB::Options {};
/// A test pass that always aborts to enable testing the crash recovery
/// mechanism of the pass manager.
struct TestCrashRecoveryPass
@ -270,6 +315,9 @@ void registerPassManagerTestPass() {
PassRegistration<TestOptionsPass>();
PassRegistration<TestOptionsSuperPass>();
PassRegistration<TestOptionsPassA>();
PassRegistration<TestOptionsPassB>();
PassRegistration<TestModulePass>();
PassRegistration<TestFunctionPass>();
@ -306,5 +354,16 @@ void registerPassManagerTestPass() {
[](OpPassManager &pm, const TestOptionsSuperPass::Options &options) {
pm.addPass(std::make_unique<TestOptionsSuperPass>(options));
});
PassPipelineRegistration<TestPipelineOptionsSuperSetAB>
registerPipelineOptionsSuperSetABPipeline(
"test-options-super-set-ab-pipeline",
"Parses options of PassPipelineOptions using pass pipeline "
"registration",
[](OpPassManager &pm, const TestPipelineOptionsSuperSetAB &options) {
// Pass superset AB options to subset options A and B
pm.addPass(std::make_unique<TestOptionsPassA>(options));
pm.addPass(std::make_unique<TestOptionsPassB>(options));
});
}
} // namespace mlir