aurel333 commented on a change in pull request #6412:
URL: https://github.com/apache/incubator-tvm/pull/6412#discussion_r486888553
##########
File path: src/runtime/opencl/opencl_common.h
##########
@@ -216,7 +216,8 @@ class OpenCLWorkspace : public DeviceAPI {
// Initialzie the device.
void Init(const std::string& type_key, const std::string& device_type,
const std::string& platform_name = "");
- virtual void Init() { Init("opencl", "gpu"); }
+ // Find accelerator first, TODO: Make a proper way to chose between device
types
+ virtual void Init() { Init("opencl", "accelerator"); }
Review comment:
I agree there needs to have a discussion. I think that putting
accelerator in the default OpenCL is a necessity because it will allow people
who use custom or currently unsupported accelerators to use TVM easily. However
because it will use CUDA schedules, the performances may suffer so maybe
putting accelerators with a low priority would be better.
At the end, the present system is in my opinion flawed because it does not
let the user chose which type of device to prioritize for OpenCL. Maybe we
should split the current OpenCL target to three (or four with custom?) targets
like "opencl-cpu", "opencl-gpu" and "opencl-accelerator". Or we could use a
target attribute to do it. What do you think about it?
##########
File path: src/runtime/opencl/opencl_common.h
##########
@@ -216,7 +216,8 @@ class OpenCLWorkspace : public DeviceAPI {
// Initialzie the device.
void Init(const std::string& type_key, const std::string& device_type,
const std::string& platform_name = "");
- virtual void Init() { Init("opencl", "gpu"); }
+ // Find accelerator first, TODO: Make a proper way to chose between device
types
+ virtual void Init() { Init("opencl", "accelerator"); }
Review comment:
I agree there needs to have a discussion. I think that putting
accelerator in the default OpenCL is a necessity because it will allow people
who use custom or currently unsupported accelerators to use TVM easily. However
because it will use CUDA schedules, the performances may suffer so maybe
putting accelerators with a low priority would be better.
At the end, the present system is in my opinion flawed because it does not
let the user chose which type of device to prioritize for OpenCL. Maybe we
should split the current OpenCL target to three (or four with custom?) targets
like "opencl-cpu", "opencl-gpu" and "opencl-accelerator". Or we could use a
target attribute to do it. What do you think about it?
##########
File path: src/runtime/opencl/opencl_common.h
##########
@@ -216,7 +216,8 @@ class OpenCLWorkspace : public DeviceAPI {
// Initialzie the device.
void Init(const std::string& type_key, const std::string& device_type,
const std::string& platform_name = "");
- virtual void Init() { Init("opencl", "gpu"); }
+ // Find accelerator first, TODO: Make a proper way to chose between device
types
+ virtual void Init() { Init("opencl", "accelerator"); }
Review comment:
I agree there needs to have a discussion. I think that putting
accelerator in the default OpenCL is a necessity because it will allow people
who use custom or currently unsupported accelerators to use TVM easily. However
because it will use CUDA schedules, the performances may suffer so maybe
putting accelerators with a low priority would be better.
At the end, the present system is in my opinion flawed because it does not
let the user chose which type of device to prioritize for OpenCL. Maybe we
should split the current OpenCL target to three (or four with custom?) targets
like "opencl-cpu", "opencl-gpu" and "opencl-accelerator". Or we could use a
target attribute to do it. What do you think about it?
##########
File path: src/runtime/opencl/opencl_common.h
##########
@@ -216,7 +216,8 @@ class OpenCLWorkspace : public DeviceAPI {
// Initialzie the device.
void Init(const std::string& type_key, const std::string& device_type,
const std::string& platform_name = "");
- virtual void Init() { Init("opencl", "gpu"); }
+ // Find accelerator first, TODO: Make a proper way to chose between device
types
+ virtual void Init() { Init("opencl", "accelerator"); }
Review comment:
I agree there needs to have a discussion. I think that putting
accelerator in the default OpenCL is a necessity because it will allow people
who use custom or currently unsupported accelerators to use TVM easily. However
because it will use CUDA schedules, the performances may suffer so maybe
putting accelerators with a low priority would be better.
At the end, the present system is in my opinion flawed because it does not
let the user chose which type of device to prioritize for OpenCL. Maybe we
should split the current OpenCL target to three (or four with custom?) targets
like "opencl-cpu", "opencl-gpu" and "opencl-accelerator". Or we could use a
target attribute to do it. What do you think about it?
##########
File path: src/runtime/opencl/opencl_common.h
##########
@@ -216,7 +216,8 @@ class OpenCLWorkspace : public DeviceAPI {
// Initialzie the device.
void Init(const std::string& type_key, const std::string& device_type,
const std::string& platform_name = "");
- virtual void Init() { Init("opencl", "gpu"); }
+ // Find accelerator first, TODO: Make a proper way to chose between device
types
+ virtual void Init() { Init("opencl", "accelerator"); }
Review comment:
I agree there needs to have a discussion. I think that putting
accelerator in the default OpenCL is a necessity because it will allow people
who use custom or currently unsupported accelerators to use TVM easily. However
because it will use CUDA schedules, the performances may suffer so maybe
putting accelerators with a low priority would be better.
At the end, the present system is in my opinion flawed because it does not
let the user chose which type of device to prioritize for OpenCL. Maybe we
should split the current OpenCL target to three (or four with custom?) targets
like "opencl-cpu", "opencl-gpu" and "opencl-accelerator". Or we could use a
target attribute to do it. What do you think about it?
##########
File path: src/runtime/opencl/opencl_common.h
##########
@@ -216,7 +216,8 @@ class OpenCLWorkspace : public DeviceAPI {
// Initialzie the device.
void Init(const std::string& type_key, const std::string& device_type,
const std::string& platform_name = "");
- virtual void Init() { Init("opencl", "gpu"); }
+ // Find accelerator first, TODO: Make a proper way to chose between device
types
+ virtual void Init() { Init("opencl", "accelerator"); }
Review comment:
I agree there needs to have a discussion. I think that putting
accelerator in the default OpenCL is a necessity because it will allow people
who use custom or currently unsupported accelerators to use TVM easily. However
because it will use CUDA schedules, the performances may suffer so maybe
putting accelerators with a low priority would be better.
At the end, the present system is in my opinion flawed because it does not
let the user chose which type of device to prioritize for OpenCL. Maybe we
should split the current OpenCL target to three (or four with custom?) targets
like "opencl-cpu", "opencl-gpu" and "opencl-accelerator". Or we could use a
target attribute to do it. What do you think about it?
##########
File path: src/runtime/opencl/opencl_common.h
##########
@@ -216,7 +216,8 @@ class OpenCLWorkspace : public DeviceAPI {
// Initialzie the device.
void Init(const std::string& type_key, const std::string& device_type,
const std::string& platform_name = "");
- virtual void Init() { Init("opencl", "gpu"); }
+ // Find accelerator first, TODO: Make a proper way to chose between device
types
+ virtual void Init() { Init("opencl", "accelerator"); }
Review comment:
I agree there needs to have a discussion. I think that putting
accelerator in the default OpenCL is a necessity because it will allow people
who use custom or currently unsupported accelerators to use TVM easily. However
because it will use CUDA schedules, the performances may suffer so maybe
putting accelerators with a low priority would be better.
At the end, the present system is in my opinion flawed because it does not
let the user chose which type of device to prioritize for OpenCL. Maybe we
should split the current OpenCL target to three (or four with custom?) targets
like "opencl-cpu", "opencl-gpu" and "opencl-accelerator". Or we could use a
target attribute to do it. What do you think about it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
