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 2020/12/05 03:14:51 UTC
[GitHub] [tvm] masahi opened a new pull request #7038: [ROCm][Auto schedular] Support Auto schedular and NHWC convolution on ROCm
masahi opened a new pull request #7038:
URL: https://github.com/apache/tvm/pull/7038
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tvm] merrymercy commented on a change in pull request #7038: [ROCm][Auto schedular] Support Auto schedular and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7038:
URL: https://github.com/apache/tvm/pull/7038#discussion_r536513796
##########
File path: src/auto_scheduler/search_task.cc
##########
@@ -66,6 +69,13 @@ HardwareParams HardwareParamsNode::GetDefaultHardwareParams(const Target& target
device_api->GetAttr(ctx, tvm::runtime::DeviceAttrKind::kMaxRegistersPerBlock, &ret);
int max_registers_per_block = ret;
Review comment:
I think the name "max_registers_per_block" in `HardwareParams` is a bug.
It should be renamed to`max_local_memory_per_block`. Because this is the only way it is used in `VerifyGPUCode` pass.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tvm] merrymercy commented on a change in pull request #7038: [ROCm][Auto schedular] Support Auto schedular and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7038:
URL: https://github.com/apache/tvm/pull/7038#discussion_r536514067
##########
File path: src/auto_scheduler/search_task.cc
##########
@@ -66,6 +69,13 @@ HardwareParams HardwareParamsNode::GetDefaultHardwareParams(const Target& target
device_api->GetAttr(ctx, tvm::runtime::DeviceAttrKind::kMaxRegistersPerBlock, &ret);
int max_registers_per_block = ret;
Review comment:
This check should not be used in my option. Autotvm does not use this check either.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tvm] masahi merged pull request #7038: [ROCm][Auto scheduler] Support Auto scheduler and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
masahi merged pull request #7038:
URL: https://github.com/apache/tvm/pull/7038
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #7038: [ROCm][Auto schedular] Support Auto schedular and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7038:
URL: https://github.com/apache/tvm/pull/7038#discussion_r536512913
##########
File path: src/auto_scheduler/search_task.cc
##########
@@ -66,6 +69,11 @@ HardwareParams HardwareParamsNode::GetDefaultHardwareParams(const Target& target
device_api->GetAttr(ctx, tvm::runtime::DeviceAttrKind::kMaxRegistersPerBlock, &ret);
int max_registers_per_block = ret;
+ if (max_registers_per_block == 0) {
+ LOG(INFO) << "Device API reported kMaxRegistersPerBlock being 0, using the value of "
+ "kMaxSharedMemoryPerBlock as max_registers_per_block.";
Review comment:
This is following mercy's suggestion https://discuss.tvm.apache.org/t/auto-schedular-performance-on-amdgpu-the-first-attempt/8582/17
Since I also don't understand this issue well, I turned it into a comment with more info.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tvm] merrymercy commented on a change in pull request #7038: [ROCm][Auto schedular] Support Auto schedular and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7038:
URL: https://github.com/apache/tvm/pull/7038#discussion_r536513796
##########
File path: src/auto_scheduler/search_task.cc
##########
@@ -66,6 +69,13 @@ HardwareParams HardwareParamsNode::GetDefaultHardwareParams(const Target& target
device_api->GetAttr(ctx, tvm::runtime::DeviceAttrKind::kMaxRegistersPerBlock, &ret);
int max_registers_per_block = ret;
Review comment:
I think the name "max_registers_per_block" in `HardwareParams` is a bug.
I will send another PR to rename `max_registers_per_block` to `max_local_memory_per_block` to make it align with `VerifyGPUCode` pass.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tvm] merrymercy commented on a change in pull request #7038: [ROCm][Auto schedular] Support Auto schedular and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7038:
URL: https://github.com/apache/tvm/pull/7038#discussion_r536513571
##########
File path: src/auto_scheduler/feature.cc
##########
@@ -1296,7 +1296,8 @@ void GetPerStoreFeaturesWorkerFunc(const SearchTask& task, const State& state, i
}
auto mod = IRModule(Map<GlobalVar, BaseFunc>({{global_var, f}}));
- if (task->target->kind->device_type == kDLGPU) {
+ auto device_type = task->target->kind->device_type;
+ if (device_type == kDLGPU || device_type == kDLROCM) {
Review comment:
To align with the search policy, we can try to the condition from this function
https://github.com/apache/tvm/blob/fd5ce645941153972ecee404c90479b2b391df15/src/auto_scheduler/search_policy/utils.h#L55-L62
##########
File path: src/auto_scheduler/search_task.cc
##########
@@ -66,6 +69,13 @@ HardwareParams HardwareParamsNode::GetDefaultHardwareParams(const Target& target
device_api->GetAttr(ctx, tvm::runtime::DeviceAttrKind::kMaxRegistersPerBlock, &ret);
int max_registers_per_block = ret;
Review comment:
I think this is a bug.
I will send another PR to rename `max_registers_per_block` to `max_local_memory_per_block` to make it align with `VerifyGPUCode` pass.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tvm] merrymercy commented on a change in pull request #7038: [ROCm][Auto schedular] Support Auto schedular and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7038:
URL: https://github.com/apache/tvm/pull/7038#discussion_r536513796
##########
File path: src/auto_scheduler/search_task.cc
##########
@@ -66,6 +69,13 @@ HardwareParams HardwareParamsNode::GetDefaultHardwareParams(const Target& target
device_api->GetAttr(ctx, tvm::runtime::DeviceAttrKind::kMaxRegistersPerBlock, &ret);
int max_registers_per_block = ret;
Review comment:
I think the name "max_registers_per_block" in `HardwareParams` is a bug.
It should be renamed to`max_local_memory_per_block`. Because this is the only way it is used in `VerifyGPUCode` pass. I can send another PR to fix 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #7038: [ROCm][Auto schedular] Support Auto schedular and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #7038:
URL: https://github.com/apache/tvm/pull/7038#discussion_r536515597
##########
File path: src/auto_scheduler/feature.cc
##########
@@ -1296,7 +1296,8 @@ void GetPerStoreFeaturesWorkerFunc(const SearchTask& task, const State& state, i
}
auto mod = IRModule(Map<GlobalVar, BaseFunc>({{global_var, f}}));
- if (task->target->kind->device_type == kDLGPU) {
+ auto device_type = task->target->kind->device_type;
+ if (device_type == kDLGPU || device_type == kDLROCM) {
Review comment:
Nice, replaced with this function to support other GPU targets.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tvm] comaniac commented on a change in pull request #7038: [ROCm][Auto schedular] Support Auto schedular and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7038:
URL: https://github.com/apache/tvm/pull/7038#discussion_r536509015
##########
File path: src/auto_scheduler/search_task.cc
##########
@@ -66,6 +69,11 @@ HardwareParams HardwareParamsNode::GetDefaultHardwareParams(const Target& target
device_api->GetAttr(ctx, tvm::runtime::DeviceAttrKind::kMaxRegistersPerBlock, &ret);
int max_registers_per_block = ret;
+ if (max_registers_per_block == 0) {
+ LOG(INFO) << "Device API reported kMaxRegistersPerBlock being 0, using the value of "
+ "kMaxSharedMemoryPerBlock as max_registers_per_block.";
Review comment:
It seems to me that most people won't understand the meaning of this message. Maybe either turning it to a comment, or mentioning this is for the performance cost model?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tvm] merrymercy commented on a change in pull request #7038: [ROCm][Auto schedular] Support Auto schedular and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7038:
URL: https://github.com/apache/tvm/pull/7038#discussion_r536514067
##########
File path: src/auto_scheduler/search_task.cc
##########
@@ -66,6 +69,13 @@ HardwareParams HardwareParamsNode::GetDefaultHardwareParams(const Target& target
device_api->GetAttr(ctx, tvm::runtime::DeviceAttrKind::kMaxRegistersPerBlock, &ret);
int max_registers_per_block = ret;
Review comment:
This check on local memory should not be used in my option. Autotvm does not use check local memory either.
##########
File path: src/auto_scheduler/search_task.cc
##########
@@ -66,6 +69,13 @@ HardwareParams HardwareParamsNode::GetDefaultHardwareParams(const Target& target
device_api->GetAttr(ctx, tvm::runtime::DeviceAttrKind::kMaxRegistersPerBlock, &ret);
int max_registers_per_block = ret;
Review comment:
The check on local memory should not be used in my option. Autotvm does not use check local memory either.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tvm] merrymercy commented on a change in pull request #7038: [ROCm][Auto schedular] Support Auto schedular and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
merrymercy commented on a change in pull request #7038:
URL: https://github.com/apache/tvm/pull/7038#discussion_r536513571
##########
File path: src/auto_scheduler/feature.cc
##########
@@ -1296,7 +1296,8 @@ void GetPerStoreFeaturesWorkerFunc(const SearchTask& task, const State& state, i
}
auto mod = IRModule(Map<GlobalVar, BaseFunc>({{global_var, f}}));
- if (task->target->kind->device_type == kDLGPU) {
+ auto device_type = task->target->kind->device_type;
+ if (device_type == kDLGPU || device_type == kDLROCM) {
Review comment:
To align with the search policy, we can use the condition from this function
https://github.com/apache/tvm/blob/fd5ce645941153972ecee404c90479b2b391df15/src/auto_scheduler/search_policy/utils.h#L55-L62
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tvm] masahi commented on pull request #7038: [ROCm][Auto scheduler] Support Auto scheduler and NHWC convolution on ROCm
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #7038:
URL: https://github.com/apache/tvm/pull/7038#issuecomment-739132169
will wait for https://github.com/apache/tvm/pull/7040
----------------------------------------------------------------
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:
users@infra.apache.org