Skip to content

[boo] introduce op registry #1078

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

Merged
merged 5 commits into from
Jul 23, 2025
Merged

[boo] introduce op registry #1078

merged 5 commits into from
Jul 23, 2025

Conversation

ftynse
Copy link
Contributor

@ftynse ftynse commented Jul 17, 2025

Introduce a registry class handling metadata classes for BOO ops. This automatically traverses the ops available in op_exports and registers them with a singleton registry class that can be later queried using the op "name". This name is supposed to be a unique string that cannot appear in the CLI syntax of another op. This is suboptimal and is difficult to ensure statically without a formal syntax, but will work as long as the syntax mirrors that of MIOpen driver.

Base automatically changed from users/ftynse/layer-norm to main July 17, 2025 13:53
@ftynse ftynse force-pushed the users/ftynse/generalize branch 2 times, most recently from 4295d73 to 8e0af50 Compare July 18, 2025 08:40
@ftynse ftynse marked this pull request as ready for review July 18, 2025 08:41
@ftynse ftynse changed the title WIP: [boo] introduce op registry [boo] introduce op registry Jul 18, 2025
@ftynse ftynse force-pushed the users/ftynse/generalize branch from 8e0af50 to 03131f3 Compare July 18, 2025 09:18
Copy link
Member

@rkayaith rkayaith left a comment

Choose a reason for hiding this comment

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

Overall looks good to me! Other than the registration mechanism, just minor comments.

Copy link
Contributor

@zjgarvey zjgarvey left a comment

Choose a reason for hiding this comment

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

Overall I think this is a welcome restructuring. I have some suggestions about the setup for registration. Let me know what you think.

ftynse added 3 commits July 23, 2025 19:31
Introduce a registry class handling metadata classes for BOO ops. This
automatically traverses the ops available in `op_exports` and registers
them with a singleton registry class that can be later queried using the
op "name". This name is supposed to be a unique string that cannot
appear in the CLI syntax of another op. This is suboptimal and is
difficult to ensure statically without a formal syntax, but will work as
long as the syntax mirrors that of MIOpen driver.

This allows a major refactoring of op exports that can now be aggregated
in a single directory and share generic implementation for benchmarking,
numerics, and cache prepopulation. Note that most of the implementation
was refactored previously leaing only shims in specific op exports.

Most of this commit is moving code around. Cache pre-population test is
updated to exercise the multi-op parser and dispatch. Driver files are
updated to use the registry instead of directly loading individual ops.
READMEs are consolidated into the top-level user-facing document and
developer-oriented document in the `driver` directory.

Signed-off-by: Alex Zinenko <git@ozinenko.com>
It is broken at IREE level

Signed-off-by: Alex Zinenko <git@ozinenko.com>
Signed-off-by: Alex Zinenko <git@ozinenko.com>
@ftynse ftynse force-pushed the users/ftynse/generalize branch from 48659c3 to 6e48c00 Compare July 23, 2025 20:00
Signed-off-by: Alex Zinenko <git@ozinenko.com>
@ftynse
Copy link
Contributor Author

ftynse commented Jul 23, 2025

PTAL, I'll move gemm separately so we don't have more merge conflicts

@ftynse ftynse merged commit fa4aeb2 into main Jul 23, 2025
12 checks passed
@ftynse ftynse deleted the users/ftynse/generalize branch July 23, 2025 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants