Skip to content

Conversation

@tqchen
Copy link
Member

@tqchen tqchen commented Jun 14, 2023

This PR updates the host function attachment to include host attribute so it can be lowered through MakePackedAPI.

@tvm-bot
Copy link
Collaborator

tvm-bot commented Jun 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

@tqchen
Copy link
Member Author

tqchen commented Jun 14, 2023

cc @Lunderberg this is needed after #14986 , we might want to revisit the invariance of MakePackedAPI require a host attribute (it makes sense in some way but might also get misuse)

@Lunderberg
Copy link
Contributor

Thank you @tqchen, and this change makes sense, and would prevent any later use of BindTarget from adding an erroneous host. I assume that this is needed for compatibility in the unity branch? (Trying to think of functionality tests that could be added on the main branch to validate this behavior.)

Regarding MakePackedAPI, I think the main issue is that MakePackedAPI needs to know both the host, so that the resulting device function can have the same host as is used by other functions in the module, and needs the device so that the correct device type can be checked in ArgBinder::BindDLTensor. I've been thinking about whether tir::Buffer could have an optional parameter for the device type, which could be populated during SplitHostDevice to provide the device type for MakePackedAPI to use.

This PR updates the host function attachment to include
host attribute so it can be lowered through MakePackedAPI.
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