echuraev commented on code in PR #12711:
URL: https://github.com/apache/tvm/pull/12711#discussion_r964381348
##########
src/runtime/contrib/clml/clml_runtime.cc:
##########
@@ -1056,11 +1055,8 @@ class CLMLRuntime : public JSONRuntimeBase {
CachedLayer layer_;
// CLML Context
CLMLInterfaceV2QCOM* h_ClmlIntf = NULL;
- cl_platform_id platform = NULL;
- cl_context context = NULL;
- cl_device_id device_id = NULL;
- cl_command_queue queue = NULL;
- std::vector<cl_event>* evts;
+ cl::OpenCLWorkspace* workspace = NULL;
+ cl::OpenCLThreadEntry* tentry = NULL;
Review Comment:
nit: Do you really need a separate variable for ThreadEntry? You can get it
anytime from `OpenCLWorkspace`.
##########
src/runtime/contrib/clml/clml_runtime.cc:
##########
@@ -286,21 +265,26 @@ class CLMLRuntime : public JSONRuntimeBase {
}
for (size_t i = 0; i < this->layer_.function.size(); ++i) {
- this->evts->resize(this->evts->size() + 1);
- cl_event* evt = &(this->evts->back());
- result = h_ClmlIntf->clEnqueueMLOpQCOM(queue, this->layer_.function[i],
- this->layer_.descriptorSet, 0,
NULL, evt);
+ if (getenv("CLML_PROFILING")) {
Review Comment:
What if we will use method
[IsProfiling](https://github.com/apache/tvm/blob/main/src/runtime/opencl/opencl_common.h#L278-L286)
instead of this variable? Or it is a possible situation that we have an OpenCL
queue created with profiling enable option, but we don't want to profile CLML?
##########
src/runtime/contrib/clml/clml_runtime.cc:
##########
@@ -253,6 +230,8 @@ class CLMLRuntime : public JSONRuntimeBase {
*/
void Run() override {
cl_int result = 0;
+ cl_command_queue queue = workspace->GetQueue(tentry->device);
+ std::vector<cl_event>* evts = &(workspace->GetEventQueue(tentry->device));
Review Comment:
Why do you need pointer on the vector with events instead of reference?
##########
src/runtime/contrib/clml/clml_runtime.cc:
##########
@@ -499,8 +484,17 @@ class CLMLRuntime : public JSONRuntimeBase {
uint32_t n, c, h, w;
};
- bool ExtensionStringPresent(cl_device_id device_id) {
+ bool ExtensionStringPresent(void) {
cl_int result = 0;
+ cl_platform_id platform;
+ cl_device_id device_id;
+ result = clGetPlatformIDs(1, &platform, NULL);
+ ICHECK(result == CL_SUCCESS) << "clGetPlatformIDs:" << result;
+ uint32_t num_devices = 0;
+ result = clGetDeviceIDs(platform, CL_DEVICE_TYPE_GPU, 0, NULL,
&num_devices);
+ ICHECK(result == CL_SUCCESS && num_devices == 1) << "clGetDeviceIDs:" <<
result;
+ result = clGetDeviceIDs(platform, CL_DEVICE_TYPE_GPU, 1, &device_id, NULL);
+ ICHECK(device_id && result == CL_SUCCESS) << "clGetDeviceIDs:" << result;
Review Comment:
Why do you need this code? You can get `device_id` from the `workspace`:
```
cl_device_id did =
workspace->devices[workspace->GetThreadEntry()->device.device_id];
```
I think that the best solution is to extend class `OpenCLWorkspace` with
method `cl_device_id GetClDeviceId(Device dev)`. This method also will do all
necessary checks. I think, implementation of this method will be similar with
implementation of
[GetQueue](https://github.com/apache/tvm/blob/main/src/runtime/opencl/opencl_common.h#L262-L268).
And after that you'll be able to get `cl_device_id` by calling this new
method.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
