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

[microNPU] Support different constant datatypes #9626

Merged
merged 3 commits into from
Dec 7, 2021

Conversation

lhutton1
Copy link
Contributor

@lhutton1 lhutton1 commented Dec 1, 2021

Currently only uint8 datatype is supported for constants, as this is all that was necessary until now. This PR allows different datatypes to be used for constants, including different datatypes within the same graph.

A workaround was previously added for Mean legalization, this has also been removed and replaced with the expected datatype of the constant.

Note: this PR is dependent on #9576 so contains the contents of that PR also.

cc @manupa-arm @ekalda @mbaret @NicolaLancellotti @dchauhan-arm

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! Just some comments from the 'enlightenment' category

Copy link
Contributor Author

@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.

Thanks for the review @ekalda!

@lhutton1 lhutton1 force-pushed the different-constant-data-types branch from d49e9e1 to 130ce9c Compare December 3, 2021 16:34
Currently only uint8 datatype is supported for constants, as this is
all that was necessary until now. This PR allows different datatypes
to be used for constants, including different datatypes within the
same graph.

A workaround was previously added for Mean legalization, this has
also been removed and replaced with the expected datatype of the
constant.

Change-Id: I99e34fe17905b1bb7d916e346cebfc324e3a2a0c
Change-Id: I8432f3d9dcc1001bbad40b76127075a1019197d4
Change-Id: I9b8b9df3af27023f60ef4934a918d40140a9534f
@lhutton1 lhutton1 force-pushed the different-constant-data-types branch from 130ce9c to d785a16 Compare December 6, 2021 09:30
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.

LGTM

@manupak manupak merged commit 224f8da into apache:main Dec 7, 2021
@manupak
Copy link
Contributor

manupak commented Dec 7, 2021

This is merged! Thanks @ekalda @lhutton1 @NicolaLancellotti.

@lhutton1 lhutton1 deleted the different-constant-data-types branch December 7, 2021 14:56
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
Currently only uint8 datatype is supported for constants, as this is
all that was necessary until now. This PR allows different datatypes
to be used for constants, including different datatypes within the
same graph.

A workaround was previously added for Mean legalization, this has
also been removed and replaced with the expected datatype of the
constant.
yangulei pushed a commit to yangulei/tvm that referenced this pull request Jan 11, 2022
Currently only uint8 datatype is supported for constants, as this is
all that was necessary until now. This PR allows different datatypes
to be used for constants, including different datatypes within the
same graph.

A workaround was previously added for Mean legalization, this has
also been removed and replaced with the expected datatype of the
constant.
yangulei pushed a commit to yangulei/tvm that referenced this pull request Jan 12, 2022
Currently only uint8 datatype is supported for constants, as this is
all that was necessary until now. This PR allows different datatypes
to be used for constants, including different datatypes within the
same graph.

A workaround was previously added for Mean legalization, this has
also been removed and replaced with the expected datatype of the
constant.
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
Currently only uint8 datatype is supported for constants, as this is
all that was necessary until now. This PR allows different datatypes
to be used for constants, including different datatypes within the
same graph.

A workaround was previously added for Mean legalization, this has
also been removed and replaced with the expected datatype of the
constant.
qsqqsqqsq-intellif pushed a commit to qsqqsqqsq-intellif/tvm that referenced this pull request Apr 29, 2022
Currently only uint8 datatype is supported for constants, as this is
all that was necessary until now. This PR allows different datatypes
to be used for constants, including different datatypes within the
same graph.

A workaround was previously added for Mean legalization, this has
also been removed and replaced with the expected datatype of the
constant.
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