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/18 22:33:32 UTC
[GitHub] [tvm] kparzysz-quic opened a new pull request #9970: [Hexagon] Do not auto-build apps when building TVM
kparzysz-quic opened a new pull request #9970:
URL: https://github.com/apache/tvm/pull/9970
The Hexagon cmakes have recently become unwieldy due to a complex network of dependencies between various automatically built components. This was in large part because of trying to automatically build some apps, which then tried to build TVM runtimes again, but with their own configurations.
This patch removes the ability to automatically build any Hexagon-related apps from the main TVM build. The following cmake options are now deprecated:
- `USE_HEXAGON_LAUNCHER`
- `USE_HEXAGON_PROXY_RPC`
In order to build the binaries needed for HexagonLauncher from `tvm.contrib.hexagon`:
- Build TVM+runtime for x86, with codegen for Hexagon enabled. This can be done via `USE_HEXAGON_DEVICE=sim` or `target`.
- Build Android runtime and tvm_rpc with `-DUSE_RPC=ON`, `-DUSE_CPP_RPC=ON`, and `-DUSE_HEXAGON_RPC=ON`.
- Build Hexagon runtime with `-DUSE_HEXAGON_RPC=ON`, and `-DBUILD_STATIC_RUNTIME=ON`.
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
--
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 #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #9970:
URL: https://github.com/apache/tvm/pull/9970#discussion_r789138817
##########
File path: apps/hexagon_api/CMakeLists.txt
##########
@@ -0,0 +1,83 @@
+cmake_minimum_required(VERSION 3.2)
+
+project(hexagon_api)
+
+include(ExternalProject)
+
+# Required variables:
+# ANDROID_ABI
+# ANDROID_PLATFORM
+# USE_ANDROID_TOOLCHAIN (Android toolchain .cmake file)
+# USE_HEXAGON_ARCH
+# USE_HEXAGON_SDK
+# USE_HEXAGON_TOOLCHAIN (Path to Hexagon toolchain ending with "Tools")
+
+set(TVM_SOURCE_DIR "${CMAKE_SOURCE_DIR}/../..")
+set(HEXAGON_API_BINARY_DIR "${CMAKE_BINARY_DIR}/hexagon_rpc")
Review comment:
change HEXAGON_API_BINARY_DIR to this to be compatible with current HexagonLauncher design:
set(HEXAGON_API_BINARY_DIR "${CMAKE_BINARY_DIR}/../../../build/hexagon_rpc")
This is with the assumption of building this CmakeLists.txt under `apps/hexagon_api/build`. Not the best solution, but we need to make sure everything is copied to `build/hexagon_rpc`.
##########
File path: apps/hexagon_api/CMakeLists.txt
##########
@@ -0,0 +1,83 @@
+cmake_minimum_required(VERSION 3.2)
+
+project(hexagon_api)
+
+include(ExternalProject)
+
+# Required variables:
+# ANDROID_ABI
+# ANDROID_PLATFORM
+# USE_ANDROID_TOOLCHAIN (Android toolchain .cmake file)
+# USE_HEXAGON_ARCH
+# USE_HEXAGON_SDK
+# USE_HEXAGON_TOOLCHAIN (Path to Hexagon toolchain ending with "Tools")
+
+set(TVM_SOURCE_DIR "${CMAKE_SOURCE_DIR}/../..")
+set(HEXAGON_API_BINARY_DIR "${CMAKE_BINARY_DIR}/hexagon_rpc")
+
Review comment:
Need to create the directory. maybe add `file(MAKE_DIRECTORY ${HEXAGON_API_BINARY_DIR})`
##########
File path: apps/hexagon_api/README.md
##########
@@ -0,0 +1,54 @@
+<!--- Licensed to the Apache Software Foundation (ASF) under one -->
+<!--- or more contributor license agreements. See the NOTICE file -->
+<!--- distributed with this work for additional information -->
+<!--- regarding copyright ownership. The ASF licenses this file -->
+<!--- to you under the Apache License, Version 2.0 (the -->
+<!--- "License"); you may not use this file except in compliance -->
+<!--- with the License. You may obtain a copy of the License at -->
+
+<!--- http://www.apache.org/licenses/LICENSE-2.0 -->
+
+<!--- Unless required by applicable law or agreed to in writing, -->
+<!--- software distributed under the License is distributed on an -->
+<!--- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -->
+<!--- KIND, either express or implied. See the License for the -->
+<!--- specific language governing permissions and limitations -->
+<!--- under the License. -->
+
+# Hexagon API app
+
+This is a meta-app that build the necessary binaries for use with
+the `HexagonLauncher` utility from `tvm.contrib.hexagon`.
+
+It will build the TVM runtime for Android, the RPC server application
+for Android, and the RPC library for Hexagon with the TVM runtime for
+Hexagon built into it.
+
+## Configuration
+
+There is a set of configuration variables that are required for cmake:
+- `ANDROID_ABI`: Set this to `arm64-v8a`.
+- `ANDROID_PLATFORM`: This can be `android-28`.
+- `USE_ANDROID_TOOLCHAIN`: The path to the Android toolchain file, i.e.
+`android.toolchain.cmake`. This file is a part of the Android NDK.
+- `USE_HEXAGON_ARCH`: The version string of the Hexagon architecture
+to use, i.e. vNN. The typical setting would be `v68` or later.
+- `USE_HEXAGON_SDK`: The path to the Hexagon SDK. Set this path in such
+a way that `${USE_HEXAGON_SDK}/setup_sdk_env.source` exists.
+- `USE_HEXAGON_TOOLCHAIN`: Path to Hexagon toolchain. It can be the
+Hexagon toolchain included in the SDK, for example
+`${USE_HEXAGON_TOOLCHAIN}/tools/HEXAGON_Tools/x.y.z/Tools`. The `x.y.z`
+in the path is the toolchain version number, which is specific to the
+version of the SDK.
+
+## Build
+
+The build will create a subdirectory `hexagon_rpc` with the following
+binaries
+- `tvm_runtime.so`: TVM runtime for Android (shared library).
+- `tvm_rpc_android`: RPC server for Android.
+- `libhexagon_rpc_skel.so`: RPC library for Hexagon.
+- `libtvm_runtime.a`: TVM runtime for Hexagon (static library).
+
+The RPC library for Hexagon contains the TVM runtime, so the static
+TVM runtime for Hexagon is not strictly necessary.
Review comment:
I suggest to add an instruction like this to make sure we keeps the relative directory to output of binary files.
```
## Instruction
cd apps/hexagon_api
mkdir build
cd build
cmake -DConfig1 -DConfig2 ..
```
--
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 commented on pull request #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on pull request #9970:
URL: https://github.com/apache/tvm/pull/9970#issuecomment-1016937131
I like the idea of simplifying the build process, but I'm worried that this would make it more difficult for developers to test changes. This PR would reduce the complexity of the existing cmake setup, but I think it would result in passing that complexity to the developer to manually handle.
Build details, as I understand them.
* Three TVM builds are required. (Host, connected Android, connected Hexagon)
* Each of the three builds has different values for the RPC and static/dynamic options.
* All three builds must have the same values for several build variables.
* Hexagon: `USE_HEXAGON_SDK` and `USE_HEXAGON_ARCH`
* Android: `USE_ANDROID_TOOLCHAIN`
* All three builds must have be built from the same version of TVM.
* All three builds must output to the same location (currently `$BUILD_DIR/apps/hexagon_*`).
Brainstorming on options
* Option 1: Top-level TVM CMake launches the three independent builds as external projects
* Current implementation on main.
* Pro: Developer can run `make && run_my_test.py` to rebuild all three tests in a single step.
* Con: Difficult to manage setup.
* Option 2: Developer launches the three independent builds
* Current implementation in this PR.
* Pro: Easier to manage cmake setup.
* Con: Interactions between the three builds must be handled by each developer, either manually or by writing a wrapper build script.
* Con: Removes the app from the TVM framework. Compiling TVM is no longer sufficient to use `tvm.contrib.hexagon`.
* Option 3: App Cmake launches all three independent builds
* Pro: Developer can run tests from a single location
* Pro: Includes occur only in one direction, from the app to the TVM build.
* Con: Removes the app from the TVM framework. Compiling TVM is no longer sufficient to use `tvm.contrib.hexagon`.
Between the high-level options, I'd prefer Option 1, since it would maintain the same developer-facing interactions. Thoughts?
--
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 a change in pull request #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on a change in pull request #9970:
URL: https://github.com/apache/tvm/pull/9970#discussion_r789189110
##########
File path: apps/hexagon_api/CMakeLists.txt
##########
@@ -0,0 +1,83 @@
+cmake_minimum_required(VERSION 3.2)
+
+project(hexagon_api)
+
+include(ExternalProject)
+
+# Required variables:
+# ANDROID_ABI
+# ANDROID_PLATFORM
+# USE_ANDROID_TOOLCHAIN (Android toolchain .cmake file)
+# USE_HEXAGON_ARCH
+# USE_HEXAGON_SDK
+# USE_HEXAGON_TOOLCHAIN (Path to Hexagon toolchain ending with "Tools")
+
+set(TVM_SOURCE_DIR "${CMAKE_SOURCE_DIR}/../..")
+set(HEXAGON_API_BINARY_DIR "${CMAKE_BINARY_DIR}/hexagon_rpc")
Review comment:
This is going to be really fragile. Instead I propose a flag `USE_OUTPUT_BINARY_DIR` that could be set to any location, including `.../build/hexagon_rpc`.
--
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 #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on pull request #9970:
URL: https://github.com/apache/tvm/pull/9970#issuecomment-1016948327
I chatted with Mehrdad and we're going with an app that will build the Hexagon RPC support.
--
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 merged pull request #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
kparzysz-quic merged pull request #9970:
URL: https://github.com/apache/tvm/pull/9970
--
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 a change in pull request #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on a change in pull request #9970:
URL: https://github.com/apache/tvm/pull/9970#discussion_r789194243
##########
File path: apps/hexagon_api/README.md
##########
@@ -0,0 +1,54 @@
+<!--- Licensed to the Apache Software Foundation (ASF) under one -->
+<!--- or more contributor license agreements. See the NOTICE file -->
+<!--- distributed with this work for additional information -->
+<!--- regarding copyright ownership. The ASF licenses this file -->
+<!--- to you under the Apache License, Version 2.0 (the -->
+<!--- "License"); you may not use this file except in compliance -->
+<!--- with the License. You may obtain a copy of the License at -->
+
+<!--- http://www.apache.org/licenses/LICENSE-2.0 -->
+
+<!--- Unless required by applicable law or agreed to in writing, -->
+<!--- software distributed under the License is distributed on an -->
+<!--- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -->
+<!--- KIND, either express or implied. See the License for the -->
+<!--- specific language governing permissions and limitations -->
+<!--- under the License. -->
+
+# Hexagon API app
+
+This is a meta-app that build the necessary binaries for use with
+the `HexagonLauncher` utility from `tvm.contrib.hexagon`.
+
+It will build the TVM runtime for Android, the RPC server application
+for Android, and the RPC library for Hexagon with the TVM runtime for
+Hexagon built into it.
+
+## Configuration
+
+There is a set of configuration variables that are required for cmake:
+- `ANDROID_ABI`: Set this to `arm64-v8a`.
+- `ANDROID_PLATFORM`: This can be `android-28`.
+- `USE_ANDROID_TOOLCHAIN`: The path to the Android toolchain file, i.e.
+`android.toolchain.cmake`. This file is a part of the Android NDK.
+- `USE_HEXAGON_ARCH`: The version string of the Hexagon architecture
+to use, i.e. vNN. The typical setting would be `v68` or later.
+- `USE_HEXAGON_SDK`: The path to the Hexagon SDK. Set this path in such
+a way that `${USE_HEXAGON_SDK}/setup_sdk_env.source` exists.
+- `USE_HEXAGON_TOOLCHAIN`: Path to Hexagon toolchain. It can be the
+Hexagon toolchain included in the SDK, for example
+`${USE_HEXAGON_TOOLCHAIN}/tools/HEXAGON_Tools/x.y.z/Tools`. The `x.y.z`
+in the path is the toolchain version number, which is specific to the
+version of the SDK.
+
+## Build
+
+The build will create a subdirectory `hexagon_rpc` with the following
+binaries
+- `tvm_runtime.so`: TVM runtime for Android (shared library).
+- `tvm_rpc_android`: RPC server for Android.
+- `libhexagon_rpc_skel.so`: RPC library for Hexagon.
+- `libtvm_runtime.a`: TVM runtime for Hexagon (static library).
+
+The RPC library for Hexagon contains the TVM runtime, so the static
+TVM runtime for Hexagon is not strictly necessary.
Review comment:
I updated it with the new variable.
--
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 #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #9970:
URL: https://github.com/apache/tvm/pull/9970#discussion_r791007121
##########
File path: apps/hexagon_api/CMakeLists.txt
##########
@@ -0,0 +1,83 @@
+cmake_minimum_required(VERSION 3.2)
+
+project(hexagon_api)
+
+include(ExternalProject)
+
+# Required variables:
+# ANDROID_ABI
+# ANDROID_PLATFORM
+# USE_ANDROID_TOOLCHAIN (Android toolchain .cmake file)
+# USE_HEXAGON_ARCH
+# USE_HEXAGON_SDK
+# USE_HEXAGON_TOOLCHAIN (Path to Hexagon toolchain ending with "Tools")
+
+set(TVM_SOURCE_DIR "${CMAKE_SOURCE_DIR}/../..")
+set(HEXAGON_API_BINARY_DIR "${CMAKE_BINARY_DIR}/hexagon_rpc")
Review comment:
@kparzysz-quic makes sense. 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
[GitHub] [tvm] mehrdadh commented on a change in pull request #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
mehrdadh commented on a change in pull request #9970:
URL: https://github.com/apache/tvm/pull/9970#discussion_r791007121
##########
File path: apps/hexagon_api/CMakeLists.txt
##########
@@ -0,0 +1,83 @@
+cmake_minimum_required(VERSION 3.2)
+
+project(hexagon_api)
+
+include(ExternalProject)
+
+# Required variables:
+# ANDROID_ABI
+# ANDROID_PLATFORM
+# USE_ANDROID_TOOLCHAIN (Android toolchain .cmake file)
+# USE_HEXAGON_ARCH
+# USE_HEXAGON_SDK
+# USE_HEXAGON_TOOLCHAIN (Path to Hexagon toolchain ending with "Tools")
+
+set(TVM_SOURCE_DIR "${CMAKE_SOURCE_DIR}/../..")
+set(HEXAGON_API_BINARY_DIR "${CMAKE_BINARY_DIR}/hexagon_rpc")
Review comment:
@kparzysz-quic makes sense. 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
[GitHub] [tvm] kparzysz-quic commented on pull request #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on pull request #9970:
URL: https://github.com/apache/tvm/pull/9970#issuecomment-1015894921
cc: @mehrdadh
--
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 edited a comment on pull request #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
kparzysz-quic edited a comment on pull request #9970:
URL: https://github.com/apache/tvm/pull/9970#issuecomment-1016948327
I chatted with Mehrdad and we're going with an app that will build the Hexagon RPC support, i.e. option 3 in your list.
--
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 merged pull request #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
kparzysz-quic merged pull request #9970:
URL: https://github.com/apache/tvm/pull/9970
--
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 #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on pull request #9970:
URL: https://github.com/apache/tvm/pull/9970#issuecomment-1016617379
This replaces https://github.com/apache/tvm/pull/9904.
--
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 a change in pull request #9970: [Hexagon] Do not auto-build apps when building TVM
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on a change in pull request #9970:
URL: https://github.com/apache/tvm/pull/9970#discussion_r789193453
##########
File path: apps/hexagon_api/CMakeLists.txt
##########
@@ -0,0 +1,83 @@
+cmake_minimum_required(VERSION 3.2)
+
+project(hexagon_api)
+
+include(ExternalProject)
+
+# Required variables:
+# ANDROID_ABI
+# ANDROID_PLATFORM
+# USE_ANDROID_TOOLCHAIN (Android toolchain .cmake file)
+# USE_HEXAGON_ARCH
+# USE_HEXAGON_SDK
+# USE_HEXAGON_TOOLCHAIN (Path to Hexagon toolchain ending with "Tools")
+
+set(TVM_SOURCE_DIR "${CMAKE_SOURCE_DIR}/../..")
+set(HEXAGON_API_BINARY_DIR "${CMAKE_BINARY_DIR}/hexagon_rpc")
+
Review comment:
Done.
--
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