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

TPP Runner Wrapper pass #905

Merged
merged 2 commits into from
Apr 29, 2024
Merged

TPP Runner Wrapper pass #905

merged 2 commits into from
Apr 29, 2024

Conversation

adam-smnk
Copy link
Collaborator

Refactor MLIR Bench infrastructure into a pass.

The core logic is already covered by tpp-run tests.

@adam-smnk adam-smnk added the benchmark Triggers benchmark jobs label Apr 9, 2024
@adam-smnk
Copy link
Collaborator Author

For TPP+IMEX testing, I settled on tpp-run but running tpp-opt with desired part of the pipeline is overall cleaner.
Also, creating runner wrapper is pretty useful and best done before any other transformations. So, this refactor exposes kernel wrapper creation to tpp-opt too.

@rengolin
Copy link
Contributor

The pass name is -mlir-bench but the directory is Runner and the library built is TPPRunner. Is this a generic MLIR benchmark pass or a TPP specific thing? If the former, the naming needs to be more generic. If the latter, why change?

Do you aim to upstream this?

@adam-smnk
Copy link
Collaborator Author

Do you aim to upstream this?

No, I don't think it's generic enough for that. It stays internal for now.

If the latter, why change?

Mostly for future integration with external tools. Change is motivated by the current use case of TPP -> IMEX pipeline:
tpp-run INPUT FLAGS | sed '/Error:.*/d' | IMEX.
The error is produced because we try to run an incomplete lowering but I want to use tpp-run for simplicity.

This separate runner wrapper pass allows to retain our handy wrappers and avoid silly errors when terminating lowering pipeline early. With this PR, the following is possible:
tpp-opt -mlir-bench -default-pipeline | EXTERNAL CONSUMER

It might or might not be useful in the future. But the worst case, this change simplifies our runner a bit by moving the wrapper into a separate logic (pass).

Perhaps, we can give it a nicer name.

Copy link
Contributor

@rengolin rengolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. It's fine as is. If we want to upstream later, we can clean up.

Refactor MLIR Bench infrastructure into a pass.

The core logic is already covered by tpp-run tests.
@adam-smnk adam-smnk changed the title MLIR Bench pass TPP Runner Wrapper pass Apr 29, 2024
@adam-smnk
Copy link
Collaborator Author

NFC rebase on main + renamed to tpp-runner-wrapper

@adam-smnk adam-smnk merged commit 38807f2 into plaidml:main Apr 29, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark Triggers benchmark jobs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants