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/11/07 10:49:31 UTC
[GitHub] [tvm] srkreddy1238 opened a new pull request, #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
srkreddy1238 opened a new pull request, #13305:
URL: https://github.com/apache/tvm/pull/13305
CPP build failure for Android devices with NDK version < 23.
NDK below 23 require additional linker flags.
--
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] echuraev commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1016202247
##########
apps/cpp_rpc/CMakeLists.txt:
##########
@@ -32,6 +32,16 @@ if (OS)
endif()
endif()
+if(USE_OPENCL)
+ if (ANDROID_ABI)
+ if(DEFINED ENV{ANDROID_NDK_MAJOR1})
Review Comment:
typo: `ANDROID_NDK_MAJOR1` -> `ANDROID_NDK_MAJOR`?
--
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] lhez commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
lhez commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1015726385
##########
apps/cpp_rpc/CMakeLists.txt:
##########
@@ -32,6 +32,14 @@ if (OS)
endif()
endif()
+if(USE_OPENCL)
+ if (ANDROID_ABI)
+ if($ENV{ANDROID_NDK_MAJOR} VERSION_LESS "23")
Review Comment:
I guess this is the problem - `$ENV`.
I don't think `$ENV` is needed. The CMake toolchain file from NDK should have already defined `ANDROID_NDK_MAJOR`. Hence, when that toolchain file is specified, `ANDROID_NDK_MAJOR` can be directly used as `${ANDROID_NDK_MAJOR}`. I think certain versions of CMake don't like `$ENV` being used with `if` and `VERSION_LESS`.
@elvin-n Could you try using only `${ANDROID_NDK_MAJOR}` in the `if` statement and see if that works?
--
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] lhez commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
lhez commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1015827570
##########
apps/cpp_rpc/CMakeLists.txt:
##########
@@ -32,6 +32,14 @@ if (OS)
endif()
endif()
+if(USE_OPENCL)
Review Comment:
I have no issue with CMake 3.20.6 + NDK 21.4.7075529 using
```cmake
if(${ANDROID_NDK_MAJOR} VERSION_LESS "23")
```
Note that there is no `$ENV`. Could you give it try?
Nor did I have to specify `ANDROID_NDK_MAJOR` with this in the cmake config.
--
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] srkreddy1238 commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1016175505
##########
docker/Dockerfile.ci_adreno:
##########
@@ -25,4 +25,5 @@ COPY install/ubuntu_install_androidsdk.sh /install/ubuntu_install_androidsdk.sh
RUN bash /install/ubuntu_install_androidsdk.sh
ENV ANDROID_HOME=/opt/android-sdk-linux
ENV ANDROID_NDK_HOME=/opt/android-sdk-linux/ndk/21.3.6528147
+ENV ANDROID_NDK_MAJOR=21
Review Comment:
It depends on cmake support. In our case it doesn't.
--
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] lhez commented on pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
lhez commented on PR #13305:
URL: https://github.com/apache/tvm/pull/13305#issuecomment-1306674066
@srkreddy1238 You only need to set the toolchain file. `ANDROID_NDK_MAJOR` is automatically available in `CMakeLists.txt` once you specify the toolchain file. I think it is better to set the toolchain file location using `ENV`, not `ANDROID_NDK_MAJOR`.
--
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] srkreddy1238 commented on pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on PR #13305:
URL: https://github.com/apache/tvm/pull/13305#issuecomment-1306668369
I used ENV as in CI dockers this is the way we can supply the NDK version.
For individual compilations exporting in environment or prefixing NDK version should work.
```ANDROID_NDK_MAJOR=21 cmake -DCMAKE_TOOLCHAIN_FILE=Android/Sdk/ndk/22.1.7171670/build/cmake/android.toolchain.cmake -DANDROID_ABI=arm64-v8a -DANDROID_NATIVE_API_LEVEL=android-23 -DCMAKE_FIND_ROOT_PATH_MODE_PACKAGE=ON -DANDROID_STL=c++_static ..```
--
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] masahi merged pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
masahi merged PR #13305:
URL: https://github.com/apache/tvm/pull/13305
--
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] elvin-n commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
elvin-n commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1015469952
##########
apps/cpp_rpc/CMakeLists.txt:
##########
@@ -32,6 +32,14 @@ if (OS)
endif()
endif()
+if(USE_OPENCL)
Review Comment:
tried to build this using follow command line `cmake -DCMAKE_TOOLCHAIN_FILE=Android/Sdk/ndk/22.1.7171670/build/cmake/android.toolchain.cmake -DANDROID_ABI=arm64-v8a -DANDROID_NATIVE_API_LEVEL=android-23 -DCMAKE_FIND_ROOT_PATH_MODE_PACKAGE=ON -DANDROID_STL=c++_static ..` and got follow error:
```
CMake Error at apps/cpp_rpc/CMakeLists.txt:37 (if):
if given arguments:
"VERSION_LESS" "23"
Unknown arguments specified
```
cmake version: 3.18.4 on ubuntu 21.10
--
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] lhez commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
lhez commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1015726385
##########
apps/cpp_rpc/CMakeLists.txt:
##########
@@ -32,6 +32,14 @@ if (OS)
endif()
endif()
+if(USE_OPENCL)
+ if (ANDROID_ABI)
+ if($ENV{ANDROID_NDK_MAJOR} VERSION_LESS "23")
Review Comment:
I guess this is the problem - `$ENV`.
I don't think `$ENV` is needed. The CMake toolchain file from NDK should have already defined `ANDROID_NDK_MAJOR`. Hence, when that toolchain file is specified, `ANDROID_NDK_MAJOR` can be directly used as `${ANDROID_NDK_MAJOR}`. I think certain versions of CMake don't like `$ENV` being used with `if` and `VERSION_LESS`.
--
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] srkreddy1238 commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1016169015
##########
apps/cpp_rpc/CMakeLists.txt:
##########
@@ -32,6 +32,14 @@ if (OS)
endif()
endif()
+if(USE_OPENCL)
+ if (ANDROID_ABI)
+ if($ENV{ANDROID_NDK_MAJOR} VERSION_LESS "23")
+ set_property(TARGET tvm_rpc PROPERTY LINK_FLAGS -fuse-ld=gold)
Review Comment:
Yes, ```-fuse-ld=gold``` helps to find the lib and link it properly.
--
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] lhez commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
lhez commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1015543650
##########
apps/cpp_rpc/CMakeLists.txt:
##########
@@ -32,6 +32,14 @@ if (OS)
endif()
endif()
+if(USE_OPENCL)
Review Comment:
Does CMake 3.18.4 have a bug? [`VERSION_LESS`](https://cmake.org/cmake/help/latest/command/if.html#version-comparisons) does not seem to be a new CMake feature.
--
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] elvin-n commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
elvin-n commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1015705974
##########
apps/cpp_rpc/CMakeLists.txt:
##########
@@ -32,6 +32,14 @@ if (OS)
endif()
endif()
+if(USE_OPENCL)
Review Comment:
I agree that it must not be new, on the other hand I pointed above what I observe. Have you tried to reproduce issue on your side? Not through docker that we do not have access
--
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] echuraev commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1016156766
##########
docker/Dockerfile.ci_adreno:
##########
@@ -25,4 +25,5 @@ COPY install/ubuntu_install_androidsdk.sh /install/ubuntu_install_androidsdk.sh
RUN bash /install/ubuntu_install_androidsdk.sh
ENV ANDROID_HOME=/opt/android-sdk-linux
ENV ANDROID_NDK_HOME=/opt/android-sdk-linux/ndk/21.3.6528147
+ENV ANDROID_NDK_MAJOR=21
Review Comment:
I have doubts about explicit setting of the environment variable. As @lhez wrote, the `ANDROID_NDK_MAJOR` should be provided by NDK.
##########
apps/cpp_rpc/CMakeLists.txt:
##########
@@ -32,6 +32,14 @@ if (OS)
endif()
endif()
+if(USE_OPENCL)
+ if (ANDROID_ABI)
+ if($ENV{ANDROID_NDK_MAJOR} VERSION_LESS "23")
+ set_property(TARGET tvm_rpc PROPERTY LINK_FLAGS -fuse-ld=gold)
Review Comment:
Also, I don't understand how `-fuse-ld=gold` helps to fix your [issue](https://github.com/apache/tvm/pull/13118#issuecomment-1304477305). In the log, I see that the linker cannot find `libOpenCL.so`. By manually specifying `-fuse-ld=gold` you just force compiler to use `ld.gold` and how changing linker helps you find this library? Unfortunately, I'm not able to reproduce this issue locally with docker. I don't have access to pull the `adreno_ci` docker image.
--
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] tvm-bot commented on pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13305:
URL: https://github.com/apache/tvm/pull/13305#issuecomment-1305424308
<!---bot-comment-->
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](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @echuraev <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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] echuraev commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1016184545
##########
apps/cpp_rpc/CMakeLists.txt:
##########
@@ -32,6 +32,14 @@ if (OS)
endif()
endif()
+if(USE_OPENCL)
+ if (ANDROID_ABI)
+ if($ENV{ANDROID_NDK_MAJOR} VERSION_LESS "23")
+ set_property(TARGET tvm_rpc PROPERTY LINK_FLAGS -fuse-ld=gold)
Review Comment:
Yes, for me, it's the same: `libOpenCL.so` is not a part of the NDK. When I build `tvm_rpc` with OpenCL support than I pass explicitly path to OpenCL SDK: `-DUSE_OPENCL=<path/to/sdk>` and it works fine also with `ld`. As I have mentioned in #13118, I checked that build works on many versions of NDK: 20.0.5594570, 21.3.6528147, 21.4.7075529, 22.1.7171670, etc. And I was surprised that it doesn't work in the docker.
I'm curious why, in case of @srkreddy1238 `ld` cannot find the `libOpenCL.so` but `ld.gold` can do it. In case then linkers have the same paths, I would expect that they will work in the same way.
Anyway, I don't think that this discussion is a stopper for merging this PR. But I'm just trying to understand.
--
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] lhez commented on a diff in pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
lhez commented on code in PR #13305:
URL: https://github.com/apache/tvm/pull/13305#discussion_r1016170053
##########
apps/cpp_rpc/CMakeLists.txt:
##########
@@ -32,6 +32,14 @@ if (OS)
endif()
endif()
+if(USE_OPENCL)
+ if (ANDROID_ABI)
+ if($ENV{ANDROID_NDK_MAJOR} VERSION_LESS "23")
+ set_property(TARGET tvm_rpc PROPERTY LINK_FLAGS -fuse-ld=gold)
Review Comment:
With NDK 23 and above and `ld.gold`, the host `ld` will be used. That's why we have the failure `ld.gold: --no-rosegment: unknown option` - host `ld` doesn't have this option. My guess is `ld.gold` in older NDK may be able to get a path to `libOpenCL.so` through host `ld`. I think in @srkreddy1238 's env, `libOpenCL.so` exists in the host, but not in NDK (I don't think any NDK comes with `libOpenCL.so`).
--
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] srkreddy1238 commented on pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on PR #13305:
URL: https://github.com/apache/tvm/pull/13305#issuecomment-1305426525
This PR address the rpc build caused by https://github.com/apache/tvm/pull/13118
--
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] srkreddy1238 commented on pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on PR #13305:
URL: https://github.com/apache/tvm/pull/13305#issuecomment-1306682534
https://github.com/apache/tvm/blob/b807613c794321bf4fa765df026b1c3b454811b3/tests/scripts/task_build_adreno_bins.sh#L42
Tool chain is set.
--
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] srkreddy1238 commented on pull request #13305: [CPP_RPC][ANDROID] Fix cpp_rpc build failure
Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on PR #13305:
URL: https://github.com/apache/tvm/pull/13305#issuecomment-1306713791
made ```ANDROID_NDK_MAJOR``` to be an optional env variable to keep higher NDK versions work default.
--
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