-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[ETHOSN] Add support for non-default Ethos(TM)-N78 configurations #9386
Conversation
- Updated tvmc with new Ethos-N78 composite target. - Added additional Ethos-N78 specific configuration options. Co-authored-by: Tristan O'Connor <[email protected]>
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 something I picked up on while skimming through, will leave to others to review in more depth :)
if params: | ||
mod["main"] = bind_params_by_name(mod["main"], params) | ||
|
||
seq = tvm.transform.Sequential( | ||
[ | ||
transform.InferType(), | ||
transform.MergeComposite(pattern_table()), | ||
transform.AnnotateTarget("ethos-n"), | ||
transform.MergeCompilerRegions(), | ||
transform.PartitionGraph(), | ||
] | ||
) |
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.
This looks the same as what's in partition_for_ethosn78
? Maybe its best to wrap this in another function rather than duplicate functionality
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.
Yes, I have noticed this too, but would suggest to do this as an follow-on patch, amongst some other cleanup I want to do.
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.
Broadly looks good.
Thanks for addressing my previous comments.
I agree with @lhutton1 that we should not duplicate the common parts. Please address them in the next PR : P3 (according to the tracking issue).
Thanks @lhutton1 @Leo-arm. |
…ache#9386) - Updated tvmc with new Ethos-N78 composite target. - Added additional Ethos-N78 specific configuration options. Co-authored-by: Tristan O'Connor <[email protected]> Co-authored-by: Tristan O'Connor <[email protected]>
…ache#9386) - Updated tvmc with new Ethos-N78 composite target. - Added additional Ethos-N78 specific configuration options. Co-authored-by: Tristan O'Connor <[email protected]> Co-authored-by: Tristan O'Connor <[email protected]>
Co-authored-by: Tristan O'Connor [email protected]