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

[ETHOSN] Add support for non-default Ethos(TM)-N78 configurations #9386

Merged
merged 1 commit into from Oct 29, 2021

Conversation

Leo-arm
Copy link
Contributor

@Leo-arm Leo-arm commented Oct 28, 2021

  • Updated tvmc with new Ethos-N78 composite target.
  • Added additional Ethos-N78 specific configuration options.

Co-authored-by: Tristan O'Connor tristan.oconnor@arm.com

- Updated tvmc with new Ethos-N78 composite target.
- Added additional Ethos-N78 specific configuration options.

Co-authored-by: Tristan O'Connor <tristan.oconnor@arm.com>
Copy link
Contributor

@lhutton1 lhutton1 left a 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 :)

Comment on lines +73 to +84
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(),
]
)
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

@manupak manupak left a 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).

@manupak manupak merged commit 3b22607 into apache:main Oct 29, 2021
@manupak
Copy link
Contributor

manupak commented Oct 29, 2021

Thanks @lhutton1 @Leo-arm.

ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
…ache#9386)

- Updated tvmc with new Ethos-N78 composite target.
- Added additional Ethos-N78 specific configuration options.

Co-authored-by: Tristan O'Connor <tristan.oconnor@arm.com>

Co-authored-by: Tristan O'Connor <tristan.oconnor@arm.com>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
…ache#9386)

- Updated tvmc with new Ethos-N78 composite target.
- Added additional Ethos-N78 specific configuration options.

Co-authored-by: Tristan O'Connor <tristan.oconnor@arm.com>

Co-authored-by: Tristan O'Connor <tristan.oconnor@arm.com>
@Leo-arm Leo-arm deleted the variants branch February 21, 2022 12:25
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.

None yet

3 participants