You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/01/26 01:02:22 UTC
[GitHub] [tvm] mehrdadh opened a new pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
mehrdadh opened a new pull request #10068:
URL: https://github.com/apache/tvm/pull/10068
This PR:
- Updates documentation for building hexagon_api to run HexagonLauncher tests
- Cleanup apps/hexagon_proxy_rpc since it was a temporary solution before hexagon_api.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cconvey commented on a change in pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
cconvey commented on a change in pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#discussion_r792902407
##########
File path: tests/python/contrib/test_hexagon/rpc/test_launcher.md
##########
@@ -26,20 +26,38 @@ Here are the steps that are taken to prepare a runtime on a Hexagon device to te
- Build TVM library with Hexagon support for host machine.
- Build TVMRuntime library and C++ RPC server for host machine.
-To build these pieces, you can use a cmake command as follow.
+Note: before moving forward make sure to export Clang libraries to `LD_LIBRARY_PATH` and Hexagon toolchain to `HEXAGON_TOOLCHAIN`.
+
+To build these pieces, first build Hexagon API application under `apps/hexagon_api`.
```bash
-cmake -DUSE_HEXAGON_RPC=ON \
- -DUSE_ANDROID_TOOLCHAIN=/path/to/android-ndk/build/cmake/android.toolchain.cmake \
+cd apps/hexagon_api
+mkdir build
+cd build
+cmake -DUSE_ANDROID_TOOLCHAIN=/path/to/android-ndk/build/cmake/android.toolchain.cmake \
-DANDROID_PLATFORM=android-28 \
-DANDROID_ABI=arm64-v8a \
-DUSE_HEXAGON_ARCH=v65|v66|v68 \
-DUSE_HEXAGON_SDK=/path/to/Hexagon/SDK \
- -DUSE_HEXAGON_TOOLCHAIN=/path/to/Hexagon/toolchain/ \
- -DUSE_LLVM=/path/to/llvm/bin/llvm-config \
+ -DUSE_HEXAGON_TOOLCHAIN=/path/to/Hexagon/toolchain/"Tools"/sub-directory \
+ -DUSE_OUTPUT_BINARY_DIR=/path/to/"tvm/build/hexagon_api_output" ..
+```
+
+This command generates `tvm_rpc_android` and `libtvm_runtime.so` to run on Android. Also, it generates `libtvm_runtime.a` and `libhexagon_rpc_skel.so` to run on Hexagon device. Now we have TVM artifacts which are used to run on the remote device.
+
+Next, we need to build TVM on host with RPC and Hexagon dependencies. To do that follow these commands.
+
+```bash
+cd tvm
+mkdir build
+cd build
+cmake -DUSE_LLVM=/path/to/llvm/bin/llvm-config \
Review comment:
(Same comment as above, regarding mixing literal bash code with bash pseudo-code in a `bash`-formatted code block.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh commented on pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#issuecomment-1022375969
cc @Lunderberg
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh commented on pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#issuecomment-1022729956
@cconvey thanks for the review. I addressed your comments, PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh commented on a change in pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#discussion_r792895883
##########
File path: cmake/modules/Hexagon.cmake
##########
@@ -34,7 +34,7 @@ function(find_hexagon_toolchain)
set(TRY_PATH "${USE_HEXAGON_SDK}")
endif()
message(STATUS "Looking for Hexagon toolchain in ${TRY_PATH}")
- tvm_file_glob(GLOB_RECURSE HEXAGON_CLANG "${TRY_PATH}/*/hexagon-clang++")
+ file(GLOB_RECURSE HEXAGON_CLANG "${TRY_PATH}/*/hexagon-clang++")
Review comment:
`tvm_file_glob` is supposed to be used on TVM sub-directoy files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cconvey commented on a change in pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
cconvey commented on a change in pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#discussion_r792894545
##########
File path: cmake/modules/Hexagon.cmake
##########
@@ -34,7 +34,7 @@ function(find_hexagon_toolchain)
set(TRY_PATH "${USE_HEXAGON_SDK}")
endif()
message(STATUS "Looking for Hexagon toolchain in ${TRY_PATH}")
- tvm_file_glob(GLOB_RECURSE HEXAGON_CLANG "${TRY_PATH}/*/hexagon-clang++")
+ file(GLOB_RECURSE HEXAGON_CLANG "${TRY_PATH}/*/hexagon-clang++")
Review comment:
Just curious: why replace `tvm_file_glob` with `file`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh commented on pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#issuecomment-1022372193
@kparzysz-quic thanks for the suggestion! Fixed it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] kparzysz-quic commented on pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#issuecomment-1022287492
What happens without this hack? Are you running into errors building the Android runtime? Is it because of a missing FastRPC header? I see that on [this line](https://github.com/apache/tvm/pull/10068/files#diff-8d5cf4abb79e3cfccc22e09f139aefe541b41a7dc00db7070d20b35605eb11c1R60) you set the Hexagon device to `target`, try setting it to `sim` instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cconvey commented on a change in pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
cconvey commented on a change in pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#discussion_r792903518
##########
File path: apps/hexagon_api/CMakeLists.txt
##########
@@ -88,3 +88,6 @@ ExternalProject_Add_Step(hexagon_tvm_runtime_rpc copy_binaries
DEPENDEES install
)
+# Copy android_bash template file
Review comment:
Just a style suggestion (feel free to ignore this): The comment line seems unnecessary, because the following cmake lines are really easy to grok.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cconvey commented on a change in pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
cconvey commented on a change in pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#discussion_r792899910
##########
File path: tests/python/contrib/test_hexagon/rpc/test_launcher.md
##########
@@ -26,20 +26,38 @@ Here are the steps that are taken to prepare a runtime on a Hexagon device to te
- Build TVM library with Hexagon support for host machine.
- Build TVMRuntime library and C++ RPC server for host machine.
-To build these pieces, you can use a cmake command as follow.
+Note: before moving forward make sure to export Clang libraries to `LD_LIBRARY_PATH` and Hexagon toolchain to `HEXAGON_TOOLCHAIN`.
Review comment:
A few suggestions:
- nitpick: "before moving forward" is an unusual phrasing in a file like this, and parsing it slowed down my reading a little bit. It might be more idiomatic to say something like, "First, ensure that ...".
- For variables like these, sometimes people get confused regarding how deep into a directory structure the variables should point. It might help to give concrete examples to clarify that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh commented on a change in pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#discussion_r793119854
##########
File path: apps/hexagon_api/CMakeLists.txt
##########
@@ -88,3 +88,6 @@ ExternalProject_Add_Step(hexagon_tvm_runtime_rpc copy_binaries
DEPENDEES install
)
+# Copy android_bash template file
Review comment:
removed it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] cconvey commented on a change in pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
cconvey commented on a change in pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#discussion_r792901899
##########
File path: tests/python/contrib/test_hexagon/rpc/test_launcher.md
##########
@@ -26,20 +26,38 @@ Here are the steps that are taken to prepare a runtime on a Hexagon device to te
- Build TVM library with Hexagon support for host machine.
- Build TVMRuntime library and C++ RPC server for host machine.
-To build these pieces, you can use a cmake command as follow.
+Note: before moving forward make sure to export Clang libraries to `LD_LIBRARY_PATH` and Hexagon toolchain to `HEXAGON_TOOLCHAIN`.
+
+To build these pieces, first build Hexagon API application under `apps/hexagon_api`.
```bash
-cmake -DUSE_HEXAGON_RPC=ON \
- -DUSE_ANDROID_TOOLCHAIN=/path/to/android-ndk/build/cmake/android.toolchain.cmake \
+cd apps/hexagon_api
+mkdir build
+cd build
+cmake -DUSE_ANDROID_TOOLCHAIN=/path/to/android-ndk/build/cmake/android.toolchain.cmake \
Review comment:
Just a personal opinion, but I'd be careful with this style. It's formatted as `bash` code, and the first few lines look like literal bash code. So some people (probably me included) will potentially copy this whole block and paste it into a terminal, without reading it carefully. That's a problem because the following lines aren't _literal_ bash commands, but rather human-readable bash pseudocode.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Lunderberg merged pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
Lunderberg merged pull request #10068:
URL: https://github.com/apache/tvm/pull/10068
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh commented on pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#issuecomment-1021755789
@kparzysz-quic appreciate your review here. Also, I need to fix this hack before merge: https://github.com/apache/tvm/pull/10068/files#diff-1abd6a553915398cb46fbd7115d3427b8e87514da965925c8f31f141f86fc3a0R501
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh commented on a change in pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#discussion_r793159258
##########
File path: tests/python/contrib/test_hexagon/rpc/test_launcher.md
##########
@@ -26,20 +26,38 @@ Here are the steps that are taken to prepare a runtime on a Hexagon device to te
- Build TVM library with Hexagon support for host machine.
- Build TVMRuntime library and C++ RPC server for host machine.
-To build these pieces, you can use a cmake command as follow.
+Note: before moving forward make sure to export Clang libraries to `LD_LIBRARY_PATH` and Hexagon toolchain to `HEXAGON_TOOLCHAIN`.
+
+To build these pieces, first build Hexagon API application under `apps/hexagon_api`.
```bash
-cmake -DUSE_HEXAGON_RPC=ON \
- -DUSE_ANDROID_TOOLCHAIN=/path/to/android-ndk/build/cmake/android.toolchain.cmake \
+cd apps/hexagon_api
+mkdir build
+cd build
+cmake -DUSE_ANDROID_TOOLCHAIN=/path/to/android-ndk/build/cmake/android.toolchain.cmake \
-DANDROID_PLATFORM=android-28 \
-DANDROID_ABI=arm64-v8a \
-DUSE_HEXAGON_ARCH=v65|v66|v68 \
-DUSE_HEXAGON_SDK=/path/to/Hexagon/SDK \
- -DUSE_HEXAGON_TOOLCHAIN=/path/to/Hexagon/toolchain/ \
- -DUSE_LLVM=/path/to/llvm/bin/llvm-config \
+ -DUSE_HEXAGON_TOOLCHAIN=/path/to/Hexagon/toolchain/"Tools"/sub-directory \
+ -DUSE_OUTPUT_BINARY_DIR=/path/to/"tvm/build/hexagon_api_output" ..
+```
+
+This command generates `tvm_rpc_android` and `libtvm_runtime.so` to run on Android. Also, it generates `libtvm_runtime.a` and `libhexagon_rpc_skel.so` to run on Hexagon device. Now we have TVM artifacts which are used to run on the remote device.
+
+Next, we need to build TVM on host with RPC and Hexagon dependencies. To do that follow these commands.
+
+```bash
+cd tvm
+mkdir build
+cd build
+cmake -DUSE_LLVM=/path/to/llvm/bin/llvm-config \
Review comment:
changed the style.
##########
File path: tests/python/contrib/test_hexagon/rpc/test_launcher.md
##########
@@ -26,20 +26,38 @@ Here are the steps that are taken to prepare a runtime on a Hexagon device to te
- Build TVM library with Hexagon support for host machine.
- Build TVMRuntime library and C++ RPC server for host machine.
-To build these pieces, you can use a cmake command as follow.
+Note: before moving forward make sure to export Clang libraries to `LD_LIBRARY_PATH` and Hexagon toolchain to `HEXAGON_TOOLCHAIN`.
+
+To build these pieces, first build Hexagon API application under `apps/hexagon_api`.
```bash
-cmake -DUSE_HEXAGON_RPC=ON \
- -DUSE_ANDROID_TOOLCHAIN=/path/to/android-ndk/build/cmake/android.toolchain.cmake \
+cd apps/hexagon_api
+mkdir build
+cd build
+cmake -DUSE_ANDROID_TOOLCHAIN=/path/to/android-ndk/build/cmake/android.toolchain.cmake \
Review comment:
I agree with you. Changed the style, PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mehrdadh commented on a change in pull request #10068: [Hexagon] Update hexagon API build instruction and cleanup hexagon_proxy_rpc
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #10068:
URL: https://github.com/apache/tvm/pull/10068#discussion_r793156743
##########
File path: tests/python/contrib/test_hexagon/rpc/test_launcher.md
##########
@@ -26,20 +26,38 @@ Here are the steps that are taken to prepare a runtime on a Hexagon device to te
- Build TVM library with Hexagon support for host machine.
- Build TVMRuntime library and C++ RPC server for host machine.
-To build these pieces, you can use a cmake command as follow.
+Note: before moving forward make sure to export Clang libraries to `LD_LIBRARY_PATH` and Hexagon toolchain to `HEXAGON_TOOLCHAIN`.
Review comment:
added more details. thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org