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

[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