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

[PASS] Refactor a couple of TIR passes - BindTarget, AnnotateEntryFunc, Filter, LowerInitBlock #11628

Merged
merged 4 commits into from
Jun 9, 2022

Conversation

sunggg
Copy link
Contributor

@sunggg sunggg commented Jun 8, 2022

This PR fixes a few inconsistent pass registration and add testcases for them.

  • LowerInitBlock had mismatch between its pass name and ffi key.
  • BindTarget, AnnotateEntryFunc, Filter were not following the name convention of tir passes and they were not registered in FFI registry.

cc. @junrushao1994

@@ -24,6 +24,7 @@
#ifndef TVM_TIR_TRANSFORM_H_
#define TVM_TIR_TRANSFORM_H_

#include <tvm/driver/driver_api.h>
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible that we don't depend on driver_api.h?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing out! Changed to #include <tvm/target/target.h> which is more accurate.

*/

/*!
* \file helpers.cc
Copy link
Member

Choose a reason for hiding this comment

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

we might want to find better names other than helpers.cc. For example, driver_api_pass.cc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. How about the primfunc_utils.cc?

Copy link
Member

Choose a reason for hiding this comment

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

that sounds a reasonable choice!

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

Overall the PR looks in a good state! Please address my nitpicks and let's get it merged sooooon!

@junrushao junrushao merged commit 87502dd into apache:main Jun 9, 2022
Kathryn-cat pushed a commit to Kathryn-cat/tvm that referenced this pull request Jun 10, 2022
…c, Filter, LowerInitBlock (apache#11628)

This PR fixes a few inconsistent pass registration and add testcases for them. 
- `LowerInitBlock` had mismatch between its pass name and ffi key.
- `BindTarget`, `AnnotateEntryFunc`, `Filter` were not following the name convention of tir passes and they were not registered in FFI registry.
juda pushed a commit to juda/tvm that referenced this pull request Jun 21, 2022
…c, Filter, LowerInitBlock (apache#11628)

This PR fixes a few inconsistent pass registration and add testcases for them. 
- `LowerInitBlock` had mismatch between its pass name and ffi key.
- `BindTarget`, `AnnotateEntryFunc`, `Filter` were not following the name convention of tir passes and they were not registered in FFI registry.
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.

2 participants