Skip to content

Conversation

@Lunderberg
Copy link
Contributor

Previously, constants for ethos-u were tracked using a function attribute "ethos-u.constants". This predates the introduction of AllocateConst, and had comments indicating that it should be replaced with AllocateConst when possible.

To minimize impact to existing passes, this commit preserves the "ethos-u.constants" attribute during ethosu-specific lowering passes. The attribute is converted to AllocateConst at the end of the lower_ethosu pass, just prior to lowering with the usual TIR passes.

@tvm-bot
Copy link
Collaborator

tvm-bot commented Jul 12, 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.

  • No users to tag found in teams: ethos-u See #10317 for details

Generated by tvm-bot

@Lunderberg
Copy link
Contributor Author

This change originated while debugging #14985, but ended up being unnecessary for it, and is therefore kept as a separate PR.

@Lunderberg Lunderberg force-pushed the ethosu_allocate_const branch from 3af8087 to 35ddc1a Compare July 12, 2023 15:25
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 for these improvements @Lunderberg! Looks like it needs a rebase and I'm not sure the changes to codegen_c_host.cc and split_host_device.cc were intentional for this this patch, but otherwise looks all good to me!

@Lunderberg
Copy link
Contributor Author

Thank you for catching the extra changes in this PR. The codegen_c_host.cc changes definitely don't belong here, and are removed. The split_host_device.cc can probably be removed, as they're more closely related to #14985.

@Lunderberg Lunderberg force-pushed the ethosu_allocate_const branch from 5133f46 to 77a50e9 Compare August 1, 2023 16:53
Previously, constants for ethos-u were tracked using a function
attribute `"ethos-u.constants"`.  This predates the introduction of
`AllocateConst`, and had comments indicating that it should be
replaced with `AllocateConst` when possible.

To minimize impact to existing passes, this commit preserves the
`"ethos-u.constants"` attribute during ethosu-specific lowering
passes.  The attribute is converted to `AllocateConst` at the end of
the `lower_ethosu` pass, just prior to lowering with the usual TIR
passes.
@Lunderberg Lunderberg force-pushed the ethosu_allocate_const branch from 2c8b757 to f1a16c8 Compare September 26, 2023 18:46
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.

3 participants