-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[microTVM] tuning on micro targets with meta-schedule #13514
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
|
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
341cecf to
bc41156
Compare
zxybazh
left a comment
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.
Thanks for send the PR for micro targets MS tuning! Generally looks ok to me except a few spots needs clarification and minor changes. Would be great if someone else could take a look at the relay executor and tvm rpc changes.
guberti
left a comment
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.
I'm not the right person to do an in-depth review here, but I'm really excited for these changes. Looking at this was helpful for understanding meta-schedule - I left a few nits too.
|
@tvm-bot rerun |
|
@mkatanbaf you might need to rebase with main. |
c0396fa to
fc04678
Compare
areusch
left a comment
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.
thanks @mkatanbaf , added a few more comments/questions
| mod = IRModule({"main": prim_func}) | ||
| runtime = Runtime("crt", {"system-lib": True}) | ||
| mod = RemoveWeightLayoutRewriteBlock(skip_ndarray_rewrite=True)(mod) | ||
| rt_mod = tvm_build(mod, target=target, runtime=runtime) |
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.
just to confirm--are the relay.build changes needed in this PR? if not, could we remove them until we figure out how to wrap a TIR function in a Relay function?
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.
No, they are not. We continue to use tvm_build in tuning. The changes in relay_integration.py are needed for compiling the relay program using the MetaSchedule tuning database.
bf80e6d to
afda2c1
Compare
|
LGTM, I think we can merge once tests pass. |
8322e09 to
0c09eb7
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.
Generally looking good, just one question about the schedule rule set up.
|
|
||
| Array<ScheduleRule> ScheduleRule::DefaultMicro() { | ||
| return { | ||
| ScheduleRule::AutoInline( |
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.
Is there any specific reason that ApplyCustomRule is not included as a rule here?
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.
No, there was not. I copied the rules from the Hexagon rules a while ago, and the ApplyCustomRule was not added then. I added the rule.
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.
Hi, after you add the ApplyCustomRule you may also consider to add other rules in the current Hexagon rules like InlineConstantScalars.
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.
@zxybazh I think this could be in a follow up PR since the PR has been open for a while. thanks!
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.
Thanks @zxybazh, I already added the rest of Hexagon rules except the ParallelizeVectorizeUnroll which is not needed on the micro targets.
|
@tvm-bot rerun |
adds support for tuning microTVM models using meta-schedule. Summary of the changes: adds "c" to the targets supported by meta-schedule implements a builder and runner for micro devices runs a simple tuning job for verification Co-authored-by: Mohamad <[email protected]>
adds support for tuning microTVM models using meta-schedule.
Summary of the changes: