Issue #1762: Pipelines should have one source of truth #2259
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Set pipelineList in mlir/include/DefaultPipelines.h as the only source of truth for pipeline and pass names. Update mlir/lib/Driver/Pipelines.cpp to use getEnforceRuntimeInvariantsPipeline() function from DefaultPipelines.h. Update frontend/catalyst/pipelines.py to use get_enforce_runtime_invariants_stage() function from default_pipelines nanobind module. Add fmt library dependency to mlir/lib/Driver. Add default_pipelines nanobind module to frontend/catalyst. At the moment, it only exports a couple of functions, one to get the pipeline names, and the other to get the pass names of the EnforceRuntimeInvariants stage. Move nanobind configuration code from frontend/catalyst/utils to frontend. Add catalyst.default_pipelines as a CMakeExtension to setup.py.
…ontend/ctalyst/utils/CMakeLists.txt
Update Pipelines.cpp and pipelines.py to use the new accessor functions.
github-actions
bot
added
the
external
Contributor
|
Thank you @rturrado 🎉 As discussed in the issue I'll put a hold label on this for the time being :) |
Contributor
Author
|
@dime10 yes, yes, thanks! |
Contributor
Author
|
By the way, when unifying the HLOLoweringPass passes from Pipelines.cpp and pipelines.py I found out that:
|
Contributor
Ah, thanks for reporting this! That's exactly the problem we're trying to prevent I suppose 😅 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context:
Currently we have two pipeline registries: the frontend pipeline is used by default qjit program flow, and the CPP pipeline is used by CLI and things like having stage names in the pipeline being passed into the
pipelinekwarg on qjit, e.g. in the oqd, mbqc, ppm pytests.This seems redundant, not to mention error-prone when developing and maintaining.
With more and more custom pipelines emerging and people start thinking about pipeline organization more (e.g. #1758), it would be good to have pipeline registration in just one place.
Description of the Change:
pipelineListinmlir/include/DefaultPipelines.has the only source of truth for pipeline and pass names.This pipeline list is a vector of
PipelineInfo, each of which is a pair of pipeline name and pass names.The pass names are just vectors of strings too.
mlir/lib/Driver/Pipelines.cppandfrontend/catalyst/pipelines.pyto useDefaultPipelines.h, via a few accessor functions.These were the 2 points where information was being duplicated at the moment.
In order for the Python code to use the C++ code, add a
default_pipelinesnanobind module tofrontend/catalyst.Benefits:
The definition of the pipelines and passes, used by C++ and Python code, is defined just in one point.
Possible Drawbacks:
mlir/include/Driver/DefaultPipelines.has a dependency to thefrontend. It's not a big deal but it is ugly to depend on files outside the project.Related GitHub Issues:
#1762