[GitHub] [incubator-tvm] t-vi commented on a change in pull request #4305: Proper Device Attribute Query for AMD GPU

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services