Skip to content

Conversation

@Aleksei-grovety
Copy link
Contributor

Add support for legalizing Softmax int8 to list NPU operations as implemented in Vela.

cc @neildhickey, @lhutton1, @ekalda, @leandron, @ilyag-grovety

Add support for legalizing Softmax int8 to list NPU operations as implemented in Vela.
@tvm-bot
Copy link
Collaborator

tvm-bot commented Apr 14, 2023

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

"reshape",
"reshape",
"contrib.ethosu.pooling",
"contrib.ethosu.binary_elementwise",
Copy link
Contributor

Choose a reason for hiding this comment

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

To make a test more precise, I think it's also better to verify operator_type of each contrib.ethosu.binary_elementwise op. And maybe a corresponding parameter for pooling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add it.

Copy link
Contributor

@ekalda ekalda left a comment

Choose a reason for hiding this comment

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

Thanks @Aleksei-grovety, looks good to me in general (bar @ilyag-grovety suggestion). Since it is a 500 line beast of an operator, I wonder if it would make sense to break it out into a separate file?

@Aleksei-grovety
Copy link
Contributor Author

Thanks @Aleksei-grovety, looks good to me in general (bar @ilyag-grovety suggestion). Since it is a 500 line beast of an operator, I wonder if it would make sense to break it out into a separate file?

I will add a separate file for Softmax legalization.

@Aleksei-grovety
Copy link
Contributor Author

Aleksei-grovety commented Apr 19, 2023

There is a problem with run microTVM demos, which consists in a greatly increased time to execute lower_to_te, I'm trying to find out the reason for this.

Copy link
Contributor

@ekalda ekalda left a comment

Choose a reason for hiding this comment

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

LGTM!

@ekalda ekalda merged commit 1a70083 into apache:main Apr 26, 2023
@ekalda
Copy link
Contributor

ekalda commented Apr 26, 2023

Thanks @Aleksei-grovety and @ilyag-grovety, this is now merged!

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.

4 participants