-
Notifications
You must be signed in to change notification settings - Fork 63
[tuner] retire op_matchers.py through IREE python bindings #1264
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
Conversation
a84f7f3
to
c910188
Compare
c910188
to
8d34f9d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work so far! A couple of higher level comments:
The python bindings are a bit more general than the matching we previously had, so they will be able to handle more operation types than we used to (named ops, different conv layouts, etc.). I think it is probably better to keep this PR largely NFC in terms of op support, so lets restrict the parsers to only the types of ops that we were originally supporting. Then, as a followup, support can be added for more types of contraction/convolution ops.
This way, we keep the PRs smaller, and we can land this without needing to solve any issues with additional op support.
dc79168
to
643dc1a
Compare
643dc1a
to
3b3aee8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! It's awesome to be able to remove the op_matchers file now :)
84b198f
to
ac6b576
Compare
Signed-off-by: Bangtian Liu <[email protected]>
Signed-off-by: Bangtian Liu <[email protected]>
Signed-off-by: Bangtian Liu <[email protected]>
ac6b576
to
e7f6886
Compare
cc @kuhar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I love seeing all the deleted code in red.
Per suggestion from this comment #1264 (comment), this PR mainly removes the todo comment and adds tests for named ops. --------- Signed-off-by: Bangtian Liu <[email protected]>
As llvm/llvm-project#136054 is landed, the Python binding for `linalgOp.getIndexingMaps` can be used for tuner according to the comment #1264 (comment). Signed-off-by: Bangtian Liu <[email protected]>
Per suggestion from this comment nod-ai#1264 (comment), this PR mainly removes the todo comment and adds tests for named ops. --------- Signed-off-by: Bangtian Liu <[email protected]>
) As llvm/llvm-project#136054 is landed, the Python binding for `linalgOp.getIndexingMaps` can be used for tuner according to the comment nod-ai#1264 (comment). Signed-off-by: Bangtian Liu <[email protected]>
This PR offloads the
op_matcher.py
logic to the IREE compiler through exposed python bindings as shown below:root_op_list = iree_codegen.get_tuner_root_ops(input_module)
linalg::isaContractionOpInterface
andlinalg::inferContractionDims
linalg::isaConvolutionOpInterface
andlinalg::inferConvolutionDims
Issue:
#1110
#814
#https://github.com/nod-ai/playbook/issues/74