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/11 09:54:09 UTC

[GitHub] [tvm] echuraev opened a new pull request, #13362: [OpenCL] Introduce OpenCL wrapper to TVM

echuraev opened a new pull request, #13362:
URL: https://github.com/apache/tvm/pull/13362

   This wrapper helps dynamically loading OpenCL library. It allows us to avoid of looking for and copying OpenCL library to host, looking for OpenCL SDK.
   
   cc: @elvin-n, @masahi, @csullivan 


-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1023165455


##########
.gitmodules:
##########
@@ -16,3 +16,6 @@
 [submodule "3rdparty/cutlass"]
 	path = 3rdparty/cutlass
 	url = https://github.com/NVIDIA/cutlass.git
+[submodule "3rdparty/OpenCL-Headers"]

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


[GitHub] [tvm] echuraev commented on a diff in pull request #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1022057781


##########
cmake/modules/OpenCL.cmake:
##########
@@ -49,12 +49,17 @@ else()
 endif(USE_AOCL)
 
 if(USE_OPENCL)
-  if (NOT OpenCL_FOUND)
-    find_package(OpenCL REQUIRED)
-  endif()
-  message(STATUS "Build with OpenCL support")
   tvm_file_glob(GLOB RUNTIME_OPENCL_SRCS src/runtime/opencl/*.cc)
-  list(APPEND TVM_RUNTIME_LINKER_LIBS ${OpenCL_LIBRARIES})
+  if (OpenCL_FOUND)
+    message(STATUS "Build with OpenCL support")

Review Comment:
   This wrapper can cover all cases. I kept this search only for case which I have described in the comment to your question about OpenCL SDK. Actually, probably we can remove search of OpenCL and use wrapper in all cases. I didn't do it only because the user might want to specify a specific version of OpenCL or OpenCL SDK and I think we should give him/her this opportunity.



-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
masahi merged PR #13362:
URL: https://github.com/apache/tvm/pull/13362


-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
elvin-n commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1023034786


##########
cmake/config.cmake:
##########
@@ -65,7 +65,7 @@ set(USE_AOCL OFF)
 # Whether enable OpenCL runtime
 #
 # Possible values:
-# - ON: enable OpenCL with cmake's auto search
+# - ON: enable OpenCL with OpenCL wrapper

Review Comment:
   let's add more wording like 
   "... OpenCL wrapper to remove dependency during build time and trigger dynamic search and loading of OpenCL in runtime"



-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
elvin-n commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1021987579


##########
cmake/modules/OpenCL.cmake:
##########
@@ -49,12 +49,17 @@ else()
 endif(USE_AOCL)
 
 if(USE_OPENCL)
-  if (NOT OpenCL_FOUND)
-    find_package(OpenCL REQUIRED)
-  endif()
-  message(STATUS "Build with OpenCL support")
   tvm_file_glob(GLOB RUNTIME_OPENCL_SRCS src/runtime/opencl/*.cc)
-  list(APPEND TVM_RUNTIME_LINKER_LIBS ${OpenCL_LIBRARIES})
+  if (OpenCL_FOUND)
+    message(STATUS "Build with OpenCL support")

Review Comment:
   What are the cases when we find opencl and link vs it? Do we assume this wrapper not cover all cases and we will need to link vs original opencl?



##########
src/runtime/opencl/opencl_wrapper/opencl_wrapper.cc:
##########
@@ -0,0 +1,551 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file opencl_wrapper.cc
+ */
+
+#define CL_TARGET_OPENCL_VERSION 120
+#include <CL/cl.h>
+#include <CL/cl_gl.h>
+
+#define DMLC_USE_LOGGING_LIBRARY <tvm/runtime/logging.h>
+#include <dlfcn.h>
+#include <tvm/runtime/logging.h>
+
+#include <vector>
+
+namespace {
+#if defined(__APPLE__) || defined(__MACOSX)
+static const std::vector<const char*> default_so_paths = {
+    "libOpenCL.so", "/System/Library/Frameworks/OpenCL.framework/OpenCL"};
+#elif defined(__ANDROID__)
+static const std::vector<const char*> default_so_paths = {
+    "libOpenCL.so",
+    "/system/lib64/libOpenCL.so",
+    "/system/vendor/lib64/libOpenCL.so",
+    "/system/vendor/lib64/egl/libGLES_mali.so",
+    "/system/vendor/lib64/libPVROCL.so",
+    "/data/data/org.pocl.libs/files/lib64/libpocl.so",
+    "/system/lib/libOpenCL.so",
+    "/system/vendor/lib/libOpenCL.so",
+    "/system/vendor/lib/egl/libGLES_mali.so",
+    "/system/vendor/lib/libPVROCL.so",
+    "/data/data/org.pocl.libs/files/lib/libpocl.so"};
+#elif defined(_WIN32)
+static const std::vector<const char*> default_so_paths = {"OpenCL.dll"};
+#elif defined(__linux__)
+static const std::vector<const char*> default_so_paths = {"libOpenCL.so",
+                                                          "/usr/lib/libOpenCL.so",
+                                                          "/usr/local/lib/libOpenCL.so",
+                                                          "/usr/local/lib/libpocl.so",
+                                                          "/usr/lib64/libOpenCL.so",
+                                                          "/usr/lib32/libOpenCL.so"};
+#endif
+
+class LibOpenCLWrapper {
+ public:
+  static LibOpenCLWrapper& getInstance() {
+    static LibOpenCLWrapper instance;
+    return instance;
+  }
+  LibOpenCLWrapper(const LibOpenCLWrapper&) = delete;
+  LibOpenCLWrapper& operator=(const LibOpenCLWrapper&) = delete;
+  void* getOpenCLFunction(const char* funcName) {
+    if (m_libHandler == nullptr) openLibOpenCL();
+    return dlsym(m_libHandler, funcName);
+  }
+
+ private:
+  LibOpenCLWrapper() {}
+  ~LibOpenCLWrapper() {
+    if (m_libHandler) dlclose(m_libHandler);
+  }
+  void openLibOpenCL() {
+    for (const auto it : default_so_paths) {
+      m_libHandler = dlopen(it, RTLD_LAZY);

Review Comment:
   this must not work on Windows. Need to branch and use LoadLibrary/GetProcAddress/FreeLibrary



##########
apps/cpp_rpc/README.md:
##########
@@ -37,7 +37,15 @@ This folder contains a simple recipe to make RPC server in c++.
   # Path to the desired C++ cross compiler
   set(CMAKE_CXX_COMPILER /path/to/cross/compiler/executable)
 ```
-- If linking against a custom device OpenCL library is needed, in the config specify the path to the OpenCL SDK containing the include/CL headers and lib/ or lib64/libOpenCL.so:
+- If you need to build cpp_rpc with OpenCL support, specify variable `USE_OPENCL` in the config:
+  ```
+  set(USE_OPENCL ON)
+  ```
+  In this case [OpenCL-wrapper](../../src/runtime/opencl/opencl_wrapper) or OpenCL installed to your system will be used.

Review Comment:
   > or OpenCL installed
   
   did you mean OpenCL SDK?
   Do we consider OpenCL SDK flow at all?



-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
elvin-n commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1022832802


##########
cmake/modules/OpenCL.cmake:
##########
@@ -49,12 +49,17 @@ else()
 endif(USE_AOCL)
 
 if(USE_OPENCL)
-  if (NOT OpenCL_FOUND)
-    find_package(OpenCL REQUIRED)
-  endif()
-  message(STATUS "Build with OpenCL support")
   tvm_file_glob(GLOB RUNTIME_OPENCL_SRCS src/runtime/opencl/*.cc)
-  list(APPEND TVM_RUNTIME_LINKER_LIBS ${OpenCL_LIBRARIES})
+  if (OpenCL_FOUND)
+    message(STATUS "Build with OpenCL support")

Review Comment:
   Eventually we decided to leave static OpenCL linking but change conditions when it is selected and when wrapper should be selected. There might be cases when dlopen is prohibited due to sandbox limitations and only libraries statically selected by loader can be opened.
   Modificatoin of USE_OPENCL logic: 
   if USE_OPENCL has ON, always use wrapper and dynamic loading of libOpenCL.so in runtime
   if USE_OPENCL points to openCL SDK, then use this sdk and static linking without wrapper
   



-- 
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] areusch commented on pull request #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
areusch commented on PR #13362:
URL: https://github.com/apache/tvm/pull/13362#issuecomment-1317312896

   We discussed this in the TVM community meeting this morning 
   - Initially we developed support for OpenCL without statically linking to the library; for mobile, need to manually download the SDK to your phone, and compile TVM linking against it.
   - Alternate approach: the API is stable and we don't use very many symbols; can we develop a wrapper around OpenCL that can be dynamically loaded?
   - Two options now:
     1. OPENCL is ON: cmake tries to find OpenCL SDK somewhere at compile time
     2. OpenCL path is specified: pins the headers to a specific location
   - New proposed option:
     - ON now means to use the wrapper in this PR to automatically find the library at runtime and load it dynamically
     - approach is also used in TFLite
   - @areusch: can we detect the case where, at runtime, the user is using a version of libOpenCL.so that is too old?
     - Currently we return a call-specific error code; this is tricky
     - @echuraev: Yeah we can do this
   - The way in which we've included OpenCL headers into 3rdparty is ok.


-- 
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] driazati commented on a diff in pull request #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
driazati commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1023122538


##########
.gitmodules:
##########
@@ -16,3 +16,6 @@
 [submodule "3rdparty/cutlass"]
 	path = 3rdparty/cutlass
 	url = https://github.com/NVIDIA/cutlass.git
+[submodule "3rdparty/OpenCL-Headers"]

Review Comment:
   Can you add it to https://github.com/apache/tvm/blob/main/LICENSE as well?



-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
elvin-n commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1021996120


##########
apps/android_camera/app/src/main/jni/Application.mk:
##########
@@ -32,9 +32,9 @@ APP_ABI ?= all
 APP_STL := c++_shared
 
 APP_CPPFLAGS += -DTVM_LOG_STACK_TRACE=0 -DTVM4J_ANDROID=1 -std=c++17 -Oz -frtti
-ifeq ($(USE_OPENCL), 1)
-    APP_CPPFLAGS += -DTVM_OPENCL_RUNTIME=1
-endif
+
+# OpenCL support

Review Comment:
   returned verification of USE_OPENCL :+1: 



-- 
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 pull request #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
echuraev commented on PR #13362:
URL: https://github.com/apache/tvm/pull/13362#issuecomment-1315671263

   @tvm-bot rerun


-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1021336503


##########
apps/android_camera/app/src/main/jni/Application.mk:
##########
@@ -32,9 +32,9 @@ APP_ABI ?= all
 APP_STL := c++_shared
 
 APP_CPPFLAGS += -DTVM_LOG_STACK_TRACE=0 -DTVM4J_ANDROID=1 -std=c++17 -Oz -frtti
-ifeq ($(USE_OPENCL), 1)
-    APP_CPPFLAGS += -DTVM_OPENCL_RUNTIME=1
-endif
+
+# OpenCL support

Review Comment:
   I thought that it will be better to enable OpenCL support by default for common applications such as `android_camera` or `android_rpc`. `USE_OPENCL` is specified [here](https://github.com/apache/tvm/pull/13362/files#diff-4f7fde09bab2f87e09bd09c6e4c51c95fc1720dd6917cb239cc579fc68057e82L36-L37). It is a variable which was defined in `android_camera`. I can return this variable and enable build with OpenCL by 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


[GitHub] [tvm] csullivan commented on a diff in pull request #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1021953728


##########
src/runtime/opencl/opencl_wrapper/opencl_wrapper.cc:
##########
@@ -0,0 +1,551 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file opencl_wrapper.cc
+ */

Review Comment:
   Can we note the version of opencl for which this wrapper is applicable? 



-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1022429074


##########
src/runtime/opencl/opencl_wrapper/opencl_wrapper.cc:
##########
@@ -0,0 +1,551 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file opencl_wrapper.cc
+ */

Review Comment:
   Added information about OpenCL version. Currently, this wrapper is applicable for OpenCL 1.2. Because it is version which is used in TVM. But we can easily uplift this version, then it will be necessary.



-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1021331334


##########
cmake/modules/OpenCL.cmake:
##########
@@ -50,9 +50,18 @@ endif(USE_AOCL)
 
 if(USE_OPENCL)
   if (NOT OpenCL_FOUND)
-    find_package(OpenCL REQUIRED)
+    find_package(OpenCL)

Review Comment:
   This `find_package` also set `OpenCL_LIBRARIES` this variable specifies which OpenCL library should be linked. But agree with you that the system OpenCL library should be found [here](https://github.com/apache/tvm/pull/13362/files#diff-45747f6e0c43e6e7579d2c50160a4bdd25108cf04e98dc7ed8f1b36ddcad09b1R19). 
   I will remove this condition.



-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
elvin-n commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1021213361


##########
.gitmodules:
##########
@@ -16,3 +16,6 @@
 [submodule "3rdparty/cutlass"]
 	path = 3rdparty/cutlass
 	url = https://github.com/NVIDIA/cutlass.git
+[submodule "3rdparty/OpenCL-Headers"]

Review Comment:
   OpenCL-Headers nas license Apache 2.0 that is good and can be used for TVM.
   
    @areusch Do we need to mention of including one more third party anywhere in TVM?



##########
apps/android_camera/app/src/main/jni/Application.mk:
##########
@@ -32,9 +32,9 @@ APP_ABI ?= all
 APP_STL := c++_shared
 
 APP_CPPFLAGS += -DTVM_LOG_STACK_TRACE=0 -DTVM4J_ANDROID=1 -std=c++17 -Oz -frtti
-ifeq ($(USE_OPENCL), 1)
-    APP_CPPFLAGS += -DTVM_OPENCL_RUNTIME=1
-endif
+
+# OpenCL support

Review Comment:
   it's unclear why you have removed verification of USE_OPENCL.
   BTW, what is the USE_OPENCL? Is it defined i ntvm or in android_camera?



##########
apps/android_deploy/README.md:
##########
@@ -82,28 +79,9 @@ Upload `tvmdemo-release.apk` to your Android device and install it.
 
 ### Build with OpenCL
 
-Application does not link with OpenCL library unless you configure it to. Modify JNI Makefile config `app/src/main/jni` with proper target OpenCL configuration.
-
-Here's a piece of example for `config.mk`.
-
-```makefile
-APP_ABI = arm64-v8a
-
-APP_PLATFORM = android-17
-
-# whether enable OpenCL during compile
-USE_OPENCL = 1
-
-# the additional include headers you want to add, e.g., SDK_PATH/adrenosdk/Development/Inc
-ADD_C_INCLUDES = /opt/adrenosdk-osx/Development/Inc
-
-# the additional link libs you want to add, e.g., ANDROID_LIB_PATH/libOpenCL.so
-ADD_LDLIBS = libOpenCL.so
-```
-
-Note that you should specify the correct GPU development headers for your android device. Run `adb shell dumpsys | grep GLES` to find out what GPU your android device uses. It is very likely the library (libOpenCL.so) is already present on the mobile device. For instance, I found it under `/system/vendor/lib64`. You can do `adb pull /system/vendor/lib64/libOpenCL.so ./` to get the file to your desktop.
-
-After you setup the `config.mk`, follow the instructions in [Build APK](#buildapk) to build the Android package with OpenCL flavor.
+Application is building with OpenCL support by default.
+[OpenCL-wrapper](../../src/runtime/opencl/opencl_wrapper) is used and will dynamically load OpenCL library on the device.
+If the device doesn't have OpenCL library on it, then you'll see in the runtime that OpenCL library cannot be open.

Review Comment:
   be open -> be opened



##########
cmake/modules/OpenCL.cmake:
##########
@@ -50,9 +50,18 @@ endif(USE_AOCL)
 
 if(USE_OPENCL)
   if (NOT OpenCL_FOUND)
-    find_package(OpenCL REQUIRED)
+    find_package(OpenCL)

Review Comment:
   are these changes done only for adding definition of OpenCL_INCLUDE_DIRS? in this case let's remove everything else related to finging of opencl (we do not need it) and related to linking to opencl



-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
elvin-n commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1022710053


##########
cmake/modules/OpenCL.cmake:
##########
@@ -49,12 +49,17 @@ else()
 endif(USE_AOCL)
 
 if(USE_OPENCL)
-  if (NOT OpenCL_FOUND)
-    find_package(OpenCL REQUIRED)
-  endif()
-  message(STATUS "Build with OpenCL support")
   tvm_file_glob(GLOB RUNTIME_OPENCL_SRCS src/runtime/opencl/*.cc)
-  list(APPEND TVM_RUNTIME_LINKER_LIBS ${OpenCL_LIBRARIES})
+  if (OpenCL_FOUND)
+    message(STATUS "Build with OpenCL support")

Review Comment:
   This can happen only if this specific version of OpenCL has functions which are not in the OpenCL standard. I hardly can imagine that tvm OpenCL runtime can depend on such functions. All specific can be in OpenCL extensions which can be a part of kernels, but not host API. Let's completely remove flow with static linking and depending on OpenCL SDK



-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1022052445


##########
apps/cpp_rpc/README.md:
##########
@@ -37,7 +37,15 @@ This folder contains a simple recipe to make RPC server in c++.
   # Path to the desired C++ cross compiler
   set(CMAKE_CXX_COMPILER /path/to/cross/compiler/executable)
 ```
-- If linking against a custom device OpenCL library is needed, in the config specify the path to the OpenCL SDK containing the include/CL headers and lib/ or lib64/libOpenCL.so:
+- If you need to build cpp_rpc with OpenCL support, specify variable `USE_OPENCL` in the config:
+  ```
+  set(USE_OPENCL ON)
+  ```
+  In this case [OpenCL-wrapper](../../src/runtime/opencl/opencl_wrapper) or OpenCL installed to your system will be used.

Review Comment:
   No, I mean that also it might be system OpenCL used. As far as I remember, you can install OpenCL on Ubuntu by this command: `sudo apt install ocl-icd-opencl-dev`. I'm not sure, can we name this package as OpenCL SDK or not. But anyway it should be enough to build the `cpp_rpc` for the host with OpenCL support. OpenCL will be found on this [line](https://github.com/apache/tvm/pull/13362/files#diff-45747f6e0c43e6e7579d2c50160a4bdd25108cf04e98dc7ed8f1b36ddcad09b1R19) and we won't use the wrapper at all.



-- 
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 commented on pull request #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
masahi commented on PR #13362:
URL: https://github.com/apache/tvm/pull/13362#issuecomment-1313202444

   @elvin-n Can you review this?


-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13362:
URL: https://github.com/apache/tvm/pull/13362#issuecomment-1311477144

   <!---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 @elvin-n <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] csullivan commented on a diff in pull request #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
csullivan commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1021953728


##########
src/runtime/opencl/opencl_wrapper/opencl_wrapper.cc:
##########
@@ -0,0 +1,551 @@
+/*
+ * 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.
+ */
+
+/*!
+ * \file opencl_wrapper.cc
+ */

Review Comment:
   Can we note the version of opencl this wrapper for which this wrapper is applicable? 



-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
elvin-n commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1022712134


##########
apps/cpp_rpc/README.md:
##########
@@ -37,7 +37,15 @@ This folder contains a simple recipe to make RPC server in c++.
   # Path to the desired C++ cross compiler
   set(CMAKE_CXX_COMPILER /path/to/cross/compiler/executable)
 ```
-- If linking against a custom device OpenCL library is needed, in the config specify the path to the OpenCL SDK containing the include/CL headers and lib/ or lib64/libOpenCL.so:
+- If you need to build cpp_rpc with OpenCL support, specify variable `USE_OPENCL` in the config:
+  ```
+  set(USE_OPENCL ON)
+  ```
+  In this case [OpenCL-wrapper](../../src/runtime/opencl/opencl_wrapper) or OpenCL installed to your system will be used.

Review Comment:
   Let's stick to OpenCL wrapper only. It will definitely find OpenCL installed by apt



-- 
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 pull request #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
echuraev commented on PR #13362:
URL: https://github.com/apache/tvm/pull/13362#issuecomment-1315578335

   @tvm-bot rerun


-- 
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 #13362: [OpenCL] Introduce OpenCL wrapper to TVM

Posted by GitBox <gi...@apache.org>.
echuraev commented on code in PR #13362:
URL: https://github.com/apache/tvm/pull/13362#discussion_r1023051386


##########
cmake/config.cmake:
##########
@@ -65,7 +65,7 @@ set(USE_AOCL OFF)
 # Whether enable OpenCL runtime
 #
 # Possible values:
-# - ON: enable OpenCL with cmake's auto search
+# - ON: enable OpenCL with OpenCL wrapper

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