-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[TVMC][VitisAI] Enable Vitis AI target through TVMC #7577
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general LGTM, my only concern is with the already overused term target
, that if we can find some other term to make it clear, it would be good. BTW, @jtuyls thanks for the effort to implement TVMC support.
Apart from that, some test nits.
Co-authored-by: Cody Yu <[email protected]>
Co-authored-by: Cody Yu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general LGTM, two minor comments.
Co-authored-by: Leandro Nunes <[email protected]>
@leandron Do you have any thoughts on documenting TVMC target usage?
|
Yes, I agree this is something we need to improve. Actually in general for targets in TVM it is not easy to obtain meta-information without inspecting the code. Ideally, I'd like to make the targets self-documenting, so that we don't need to update a page and keep it in sync with changes we do in the code, and users can find useful help on the I was planning to submit a separate PR, making For the inner options, such as Concretely, I don't think this should block your PR here. But certainly is a very important topic to start a broader discussion. (cc @masahi @comaniac @junrushao1994) |
I think it would be very useful to solve this target documentation issue in general and proceeding in that manner is better than relying on hard to maintain documentation. So I definitely agree this shouldn't block this PR but I think it would be good to start/continue looking at this so we can make these options transparent to the user. |
Co-authored-by: Cody Yu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please fix the CI and let's merge this PR.
@comaniac With the CI docker update and lazy loading the pyxir package also inside the the Vitis AI partitioning pass, the tests are now passing. Could you have another look? |
Thanks @jtuyls |
* Enable Vitis AI target through TVMC & change PassContext API's * Update python/tvm/contrib/target/vitis_ai.py Co-authored-by: Cody Yu <[email protected]> * Update python/tvm/contrib/target/vitis_ai.py Co-authored-by: Cody Yu <[email protected]> * Change Vitis AI API to & address comments & fix linter issues * Update docs/deploy/vitis_ai.rst Co-authored-by: Leandro Nunes <[email protected]> * Update docs/deploy/vitis_ai.rst Co-authored-by: Cody Yu <[email protected]> * Add Vitis AI initiliazation to separate init config in TVMC composite target registry * Lazy load pyxir package in Vitis AI codegen to avoid hard dependency for TVMC * Fix TVMC Vitis AI test for compiler.compile_model API change * Lazy load pyxir package in Vitis AI partitioning pass Co-authored-by: Jorn Tuyls <[email protected]> Co-authored-by: Cody Yu <[email protected]> Co-authored-by: Leandro Nunes <[email protected]>
* Enable Vitis AI target through TVMC & change PassContext API's * Update python/tvm/contrib/target/vitis_ai.py Co-authored-by: Cody Yu <[email protected]> * Update python/tvm/contrib/target/vitis_ai.py Co-authored-by: Cody Yu <[email protected]> * Change Vitis AI API to & address comments & fix linter issues * Update docs/deploy/vitis_ai.rst Co-authored-by: Leandro Nunes <[email protected]> * Update docs/deploy/vitis_ai.rst Co-authored-by: Cody Yu <[email protected]> * Add Vitis AI initiliazation to separate init config in TVMC composite target registry * Lazy load pyxir package in Vitis AI codegen to avoid hard dependency for TVMC * Fix TVMC Vitis AI test for compiler.compile_model API change * Lazy load pyxir package in Vitis AI partitioning pass Co-authored-by: Jorn Tuyls <[email protected]> Co-authored-by: Cody Yu <[email protected]> Co-authored-by: Leandro Nunes <[email protected]>
* Enable Vitis AI target through TVMC & change PassContext API's * Update python/tvm/contrib/target/vitis_ai.py Co-authored-by: Cody Yu <[email protected]> * Update python/tvm/contrib/target/vitis_ai.py Co-authored-by: Cody Yu <[email protected]> * Change Vitis AI API to & address comments & fix linter issues * Update docs/deploy/vitis_ai.rst Co-authored-by: Leandro Nunes <[email protected]> * Update docs/deploy/vitis_ai.rst Co-authored-by: Cody Yu <[email protected]> * Add Vitis AI initiliazation to separate init config in TVMC composite target registry * Lazy load pyxir package in Vitis AI codegen to avoid hard dependency for TVMC * Fix TVMC Vitis AI test for compiler.compile_model API change * Lazy load pyxir package in Vitis AI partitioning pass Co-authored-by: Jorn Tuyls <[email protected]> Co-authored-by: Cody Yu <[email protected]> Co-authored-by: Leandro Nunes <[email protected]>
* Enable Vitis AI target through TVMC & change PassContext API's * Update python/tvm/contrib/target/vitis_ai.py Co-authored-by: Cody Yu <[email protected]> * Update python/tvm/contrib/target/vitis_ai.py Co-authored-by: Cody Yu <[email protected]> * Change Vitis AI API to & address comments & fix linter issues * Update docs/deploy/vitis_ai.rst Co-authored-by: Leandro Nunes <[email protected]> * Update docs/deploy/vitis_ai.rst Co-authored-by: Cody Yu <[email protected]> * Add Vitis AI initiliazation to separate init config in TVMC composite target registry * Lazy load pyxir package in Vitis AI codegen to avoid hard dependency for TVMC * Fix TVMC Vitis AI test for compiler.compile_model API change * Lazy load pyxir package in Vitis AI partitioning pass Co-authored-by: Jorn Tuyls <[email protected]> Co-authored-by: Cody Yu <[email protected]> Co-authored-by: Leandro Nunes <[email protected]>
This adds Vitis AI as a composite target to TVMC. As we need access to the CLI options in the partitioning function for the Vitis AI target, this PR updates the TVMC compiler and tuner interface to pass those options to the partitioning functions. The Arm Compute Lib and Ethos-N partitioning functions are adjusted accordingly.
Documentation
I think we should document the use of these targets but not sure what the best place is. For example, we can add documentation to https://tvm.apache.org/docs/tutorials/get_started/tvmc_command_line_driver.html or in the target deploy and integration documentation (for example here: https://tvm.apache.org/docs/deploy/vitis_ai.html). Or we can do both?
CI
The following PR should be merged first for tests to pass: #7575
@leandron @comaniac @mbaret