Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExternalPass API #248

Closed
Danacus opened this issue Jul 15, 2023 · 4 comments
Closed

ExternalPass API #248

Danacus opened this issue Jul 15, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@Danacus
Copy link
Contributor

Danacus commented Jul 15, 2023

As mentioned in #24, support for the ExternalPass API is considered future work.

Since I wanted to explore the possibility of writing MLIR passes in Rust, I have made a working implementation of this as a prototype. I've created an ExternalPass trait with the pass callbacks as methods. Any struct that implements the ExternalPass trait can be converted into a Pass using a provided method. I would like to extend this implementation in the future to maybe even allow Rust closures to be converted into a Pass using this API (e.g. by implementing ExternalPass for any stuct that implements Fn/FnMut/FnOnce).

Would you be interested in merging such implementation? If so, I will clean up the code and create a merge request soon.

As a side note, I've been experimenting with PDL, which enables declarative passes using pattern description. Unfortunately, this is not supported by the MLIR CAPI, but beaver has extended the CAPI to enable this. I've done a similar thing and extended both mlir-sys and melior to support PDL modules and pattern sets. However, I'm not sure if it is desirable to extend mlir-sys with features that are not already part of the CAPI. I might make a separate issue for this.

@raviqqe raviqqe added the enhancement New feature or request label Jul 15, 2023
@raviqqe
Copy link
Member

raviqqe commented Jul 15, 2023

Thank you for making the issue!

Since I wanted to explore the possibility of writing MLIR passes in Rust, I have made a working implementation of this as a prototype. I've created an ExternalPass trait with the pass callbacks as methods. Any struct that implements the ExternalPass trait can be converted into a Pass using a provided method. I would like to extend this implementation in the future to maybe even allow Rust closures to be converted into a Pass using this API (e.g. by implementing ExternalPass for any stuct that implements Fn/FnMut/FnOnce).

Of course! I would love to merge such a pull request.

As a side note, I've been experimenting with PDL, which enables declarative passes using pattern description. Unfortunately, this is not supported by the MLIR CAPI, but beaver has extended the CAPI to enable this. I've done a similar thing and extended both mlir-sys and melior to support PDL modules and pattern sets. However, I'm not sure if it is desirable to extend mlir-sys with features that are not already part of the CAPI. I might make a separate issue for this.

I would love to see the implementation. But probably modifying mlir-sys is not the best way forward as the details of MLIR itself and its C API are still marked unstable. Maybe, we can post something on the LLVM discourse?

@Danacus
Copy link
Contributor Author

Danacus commented Jul 15, 2023

My code is currently on GitLab (https://gitlab.com/Danacus/melior and https://gitlab.com/Danacus/mlir-sys), but it's not ready for merging yet, and contains several other changes I made while experimenting.

I'll create a fork here on Github with only the changes related to the external pass API and make a pull requrest.

You can find the implementation of PDL here (I have not tested it yet):

I agree that adding these CAPI extensions upstream would be better, but I'm not sure how easy this would be. As an alternative, a separate mlir-sys-ext crate could be made that compiles the CAPI code using cc in the build script, similar to what I did in my mlir-sys fork. But then adding this new crate as a dependency for melior might be a little awkward and might not be the best way forward.

@raviqqe
Copy link
Member

raviqqe commented Jul 21, 2023

Closed by #251.

@raviqqe raviqqe closed this as completed Jul 21, 2023
@raviqqe
Copy link
Member

raviqqe commented Jul 21, 2023

Let's move on discussions about PDL here. #260

@raviqqe raviqqe mentioned this issue Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants