t-vi commented on a change in pull request #4305: Proper Device Attribute Query
for AMD GPU
URL: https://github.com/apache/incubator-tvm/pull/4305#discussion_r346320975
##########
File path: src/runtime/rocm/rocm_device_api.cc
##########
@@ -54,35 +52,59 @@ class ROCMDeviceAPI final : public DeviceAPI {
break;
}
case kMaxThreadsPerBlock: {
- value = 1024;
+ ROCM_CALL(
+ hipDeviceGetAttribute(&value,
hipDeviceAttributeMaxThreadsPerBlock, ctx.device_id));
break;
}
case kWarpSize: {
- value = 64;
+ ROCM_CALL(hipDeviceGetAttribute(&value, hipDeviceAttributeWarpSize,
ctx.device_id));
+ break;
+ }
+ case kMaxSharedMemoryPerBlock: {
+ ROCM_CALL(hipDeviceGetAttribute(&value,
hipDeviceAttributeMaxSharedMemoryPerBlock,
+ ctx.device_id));
break;
}
- case kMaxSharedMemoryPerBlock: return;
case kComputeVersion: {
Review comment:
So we are changing the meaning of `kComputeVersion` from the int with the
gcn arch to the string with the "cuda compute capability approximation" here.
This is cool because it moves us closer to cuda, but I wonder whether the gcn
arch (or a string like "gfx900") is closer to what we actually need. Currently
`DetectROCMComputeVersion` (in the `codegen_amdgpu.cc`) seems to choke on the
returned string (when it expected an int with the gcn arch).
Maybe it would be prudent to kComputeVersion unchanged for now and just go
with the de-hardcoding in this PR. Then we can figure out what we want to do
(amend the device querying to get the GCN arch with another parameter, return a
string like cuda but of our own information, ...).
----------------------------------------------------------------
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]
With regards,
Apache Git Services
