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

[BYOC][CONTRIB] Vitis-AI codegen integration #6343

Merged
merged 28 commits into from
Nov 6, 2020

Conversation

anilmartha
Copy link
Contributor

This PR implements the Vitis-AI codegen using the BYOC flow and enables us to offload subgraphs to FPGA DPU accelerators (cloud/edge).

Below are the features added as part of this PR

  • Annotate the graph for the given Vitis-AI DPU (Deep Learning Processing Unit) target.
  • During codegen phase, convert the relay subgraph into PyXIR and save XGraph.
  • Vitis-AI runtime supports SaveToBinary and LoadFromBinary. We save the XGraph it in our own format and serialize it in the Module by keeping track of the path to the files.
  • Tests include a complete resnet18 model test partly offloaded to PyXIR for DPU acceleration. However, we don't have access to an FPGA instance in the CI docker environment and therefore the offloaded subgraph is just executed on CPU.

This PR depends on following Vitis-AI CI contribution PR.
The RFC for this PR can be found from here.

This work is co-authored by Jorn Tuyls jornt@xilinx.com @jtuyls, Elliott Delaye elliott@xilinx.com @edelaye and Sumit Nagpal sumitn@xilinx.com @sumitn-xilinx.

@comaniac
Copy link
Contributor

Copy link
Contributor

@mbaret mbaret left a comment

Choose a reason for hiding this comment

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

A general comment would be that it might be helpful if the Python-side code was more extensively documented.

src/runtime/contrib/vitis_ai/vitis_ai_runtime.cc Outdated Show resolved Hide resolved
src/runtime/contrib/vitis_ai/vitis_ai_runtime.cc Outdated Show resolved Hide resolved
python/tvm/relay/op/contrib/vitis_ai.py Outdated Show resolved Hide resolved
python/tvm/contrib/vitis_ai_runtime.py Outdated Show resolved Hide resolved
python/tvm/contrib/target/vitis_ai.py Outdated Show resolved Hide resolved
tests/python/contrib/test_vitis_ai_codegen.py Outdated Show resolved Hide resolved
tests/python/contrib/test_vitis_ai_codegen.py Outdated Show resolved Hide resolved
tests/python/contrib/test_vitis_ai_codegen.py Outdated Show resolved Hide resolved
tests/python/contrib/test_vitis_ai_codegen.py Outdated Show resolved Hide resolved
cmake/modules/contrib/VITISAI.cmake Outdated Show resolved Hide resolved
docs/deploy/vitis_ai.rst Show resolved Hide resolved
docs/deploy/vitis_ai.rst Outdated Show resolved Hide resolved
docs/deploy/vitis_ai.rst Outdated Show resolved Hide resolved
docs/deploy/vitis_ai.rst Outdated Show resolved Hide resolved
tests/python/contrib/test_vitis_ai_codegen.py Outdated Show resolved Hide resolved
tests/python/contrib/test_vitis_ai_runtime.py Outdated Show resolved Hide resolved
tests/python/contrib/test_vitis_ai_codegen.py Outdated Show resolved Hide resolved
tests/python/contrib/test_vitis_ai_runtime.py Outdated Show resolved Hide resolved
tests/python/contrib/test_vitis_ai_runtime.py Outdated Show resolved Hide resolved
docs/deploy/vitis_ai.rst Outdated Show resolved Hide resolved
python/tvm/relay/op/contrib/vitis_ai.py Outdated Show resolved Hide resolved
python/tvm/relay/op/contrib/vitis_ai.py Outdated Show resolved Hide resolved
src/runtime/contrib/vitis_ai/vitis_ai_runtime.cc Outdated Show resolved Hide resolved
cmake/modules/contrib/VITISAI.cmake Outdated Show resolved Hide resolved
cmake/modules/contrib/VITISAI.cmake Outdated Show resolved Hide resolved
src/runtime/contrib/vitis_ai/vitis_ai_runtime.h Outdated Show resolved Hide resolved
docker/bash.sh Outdated Show resolved Hide resolved
docker/install/ubuntu_install_python.sh Outdated Show resolved Hide resolved
docs/deploy/vitis_ai.rst Show resolved Hide resolved
docs/deploy/vitis_ai.rst Outdated Show resolved Hide resolved
docs/deploy/vitis_ai.rst Outdated Show resolved Hide resolved
python/tvm/contrib/target/vitis_ai.py Show resolved Hide resolved
src/runtime/contrib/vitis_ai/vitis_ai_runtime.cc Outdated Show resolved Hide resolved
@mbaret
Copy link
Contributor

mbaret commented Sep 8, 2020

Just as a general comment, it's useful if you can mark conversations as 'resolved' once you've made the changes so it's easy to see where conversations are still taking place.

@tmoreau89
Copy link
Contributor

@zhiics @comaniac @anilmartha I'd like to revive this PR so we can consider merging it in. As @mbaret pointed, please mark your comments as resolved if they have been addressed. If anything is still outstanding, please make it clear what you'd like @anilmartha to change.

@comaniac
Copy link
Contributor

@tmoreau89 I think we are waiting for more commits and responses from @anilmartha. I've just resolved the comments that should not be the blockers. The blockers now can be summarized as follows:

@jtuyls
Copy link
Contributor

jtuyls commented Sep 22, 2020

@zhiics @comaniac @anilmartha I'd like to revive this PR so we can consider merging it in. As @mbaret pointed, please mark your comments as resolved if they have been addressed. If anything is still outstanding, please make it clear what you'd like @anilmartha to change.

@tmoreau89 @comaniac We are currently working on following pieces to be added based on the comments:

  • Serialization of Vitis-AI build info into the TVM binary stream instead of using the previous directory based approach, see comment. This is currently being verified internally. This should also address organizing Vitis-AI artifacts comment 1 and comment 2.
  • Add additional codegen tests based on comment
  • Address comment on 2 subgraphs test
  • Address renaming comment

We will add these pieces shortly and will tag reviewers to have a look.

@tqchen
Copy link
Member

tqchen commented Sep 22, 2020

please also double check the merging, as i noticed a few files get removed

Copy link
Contributor

@comaniac comaniac 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 nits. Please fix the CI errors.

python/tvm/contrib/target/vitis_ai.py Outdated Show resolved Hide resolved
python/tvm/contrib/target/vitis_ai.py Outdated Show resolved Hide resolved
src/runtime/contrib/vitis_ai/vitis_ai_runtime.cc Outdated Show resolved Hide resolved
@tqchen tqchen changed the base branch from master to main October 11, 2020 18:21
Copy link
Member

@zhiics zhiics left a comment

Choose a reason for hiding this comment

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

just a few minor comments. Please fix the CI.

@tmoreau89, @mbaret, @jtuyls please take another look.

namespace runtime {

/*! \brief The target Vitis-AI accelerator device */
TVM_REGISTER_PASS_CONFIG_OPTION("relay.ext.vitis_ai.options.target", String);
Copy link
Member

Choose a reason for hiding this comment

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

I think we are kind of relying on the PASS_CONFIG too much. It's okay to keep it for now. We will need to figure out a better way to add these configure since they are not really related to passes.

Copy link
Contributor

@mbaret mbaret left a comment

Choose a reason for hiding this comment

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

This is looking much better, approved.

Copy link
Contributor

@tmoreau89 tmoreau89 left a comment

Choose a reason for hiding this comment

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

Thanks; nice work LGTM

@zhiics
Copy link
Member

zhiics commented Oct 21, 2020

@anilmartha please fix the CI error. Do we need to update the docker image? If that is the case, we need to bump up the image version as well.

@anilmartha
Copy link
Contributor Author

@zhiics
The CI will pass once we have updated CI with latest PyXIR version. For the same raised #6716.

@anilmartha
Copy link
Contributor Author

we have made some small changes to make the tests pass.
@zhiics @comaniac @tmoreau89 @mbaret Could you have another look?

@zhiics zhiics merged commit 3bfe6d3 into apache:main Nov 6, 2020
@zhiics
Copy link
Member

zhiics commented Nov 6, 2020

Thanks everyone. This is now merged!

@zhiics zhiics added status: accepted and removed status: need update need update based on feedbacks status: review in progress labels Nov 6, 2020
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Dec 2, 2020
* [BYOC][CONTRIB] VITIS-AI integration

* Remove environment related files

* Update vitis_ai.rst

* Add review changes

* Remove new lines and note frame in vitis_ai.rst

* use sys.exit

* Add condition for vitis_ai runtime exec function

* remove unused graph_json

* correct indentation

* use code python instead of bash

* Rename VITISAI.cmake to VitisAI.cmake

* use relay.ext.vitis_ai.options.build_dir in comparison

* Re-add deleted docker related files

* Make use of PyXIR XGraph and RuntimeModule serialization & refactor flow

* Fix linter errors

* Fix linter errors

* Address sphinx warnings

* Add infertype to fix Vitis-AI annotation test

* Renaming util to utils

* Add Vitis-AI flag to config.cmake file

* Move vitis-ai config options to compiler sources instead of runtime sources

* Fix clang-format errors

Co-authored-by: Anil Martha <anil.martha@xilinx.com>
Co-authored-by: anilm (generated by with_the_same_user script) <anilm@xhdabidk40.xilinx.com>
Co-authored-by: Jorn Tuyls <jornt@xilinx.com>
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Dec 4, 2020
* [BYOC][CONTRIB] VITIS-AI integration

* Remove environment related files

* Update vitis_ai.rst

* Add review changes

* Remove new lines and note frame in vitis_ai.rst

* use sys.exit

* Add condition for vitis_ai runtime exec function

* remove unused graph_json

* correct indentation

* use code python instead of bash

* Rename VITISAI.cmake to VitisAI.cmake

* use relay.ext.vitis_ai.options.build_dir in comparison

* Re-add deleted docker related files

* Make use of PyXIR XGraph and RuntimeModule serialization & refactor flow

* Fix linter errors

* Fix linter errors

* Address sphinx warnings

* Add infertype to fix Vitis-AI annotation test

* Renaming util to utils

* Add Vitis-AI flag to config.cmake file

* Move vitis-ai config options to compiler sources instead of runtime sources

* Fix clang-format errors

Co-authored-by: Anil Martha <anil.martha@xilinx.com>
Co-authored-by: anilm (generated by with_the_same_user script) <anilm@xhdabidk40.xilinx.com>
Co-authored-by: Jorn Tuyls <jornt@xilinx.com>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Dec 4, 2020
* [BYOC][CONTRIB] VITIS-AI integration

* Remove environment related files

* Update vitis_ai.rst

* Add review changes

* Remove new lines and note frame in vitis_ai.rst

* use sys.exit

* Add condition for vitis_ai runtime exec function

* remove unused graph_json

* correct indentation

* use code python instead of bash

* Rename VITISAI.cmake to VitisAI.cmake

* use relay.ext.vitis_ai.options.build_dir in comparison

* Re-add deleted docker related files

* Make use of PyXIR XGraph and RuntimeModule serialization & refactor flow

* Fix linter errors

* Fix linter errors

* Address sphinx warnings

* Add infertype to fix Vitis-AI annotation test

* Renaming util to utils

* Add Vitis-AI flag to config.cmake file

* Move vitis-ai config options to compiler sources instead of runtime sources

* Fix clang-format errors

Co-authored-by: Anil Martha <anil.martha@xilinx.com>
Co-authored-by: anilm (generated by with_the_same_user script) <anilm@xhdabidk40.xilinx.com>
Co-authored-by: Jorn Tuyls <jornt@xilinx.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants