[libomptarget][nfc][amdgpu] Factor out setting upper bounds

[JonChesterfield]

Summary

Refactor suggested in D103037 to help avoid similar copy-paste errors.
Change is mechanical. Some parts of this would be more robust with unsigned.

[JonChesterfield]

openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

782

minor change here, previously a single error message was used for either case.

[dhruvachak]

openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

716

How about making the change that clang-tidy is suggesting here?

Another question: Any specific reason why you are not using std::min? Is it because emitting the DP becomes a bit cumbersome?

[JonChesterfield]

openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

716

Pretty much. min doesn't say whether it made a change or not, and we've tied debug printing to whether a change was made.

I can't think of a reason why else after a return would be considered bad. Will try google on that term.

[JonChesterfield]

• twist code around clang-tidy

openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

716

...advises to reduce indentation where possible and where it makes understanding code easier. Early exit is one of the suggested enforcements of that. Please do not use else or else if after something that interrupts control flow

That seems uncompelling in this case, but it's trivial to shuffle the code to avoid the diagnostic. E.g. latest version.

[dhruvachak]

LGTM. Thanks.

openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <dlfcn.h>
 #include <elf.h>
 #include <ffi.h>
 #include <fstream>
 #include <iostream>
 #include <libelf.h>
 #include <list>
 #include <memory>
 #include <mutex>
 #include <shared_mutex>
 #include <thread>
 #include <unordered_map>
 #include <vector>
 

 int __tgt_rtl_number_of_devices() { return DeviceInfo.NumberOfDevices; }
 
 int64_t __tgt_rtl_init_requires(int64_t RequiresFlags) {
   DP("Init requires flags to %ld\n", RequiresFlags);
   DeviceInfo.RequiresFlags = RequiresFlags;
   return RequiresFlags;
 }
 
+namespace {
+template <typename T> bool enforce_upper_bound(T *value, T upper) {
+  if (*value > upper) {
+    *value = upper;
+    return true;
+  } else {
+    return false;
+  }
+}
+} // namespace
+
 int32_t __tgt_rtl_init_device(int device_id) {
   hsa_status_t err;
 
   // this is per device id init
   DP("Initialize the device id: %d\n", device_id);
 
   hsa_agent_t agent = DeviceInfo.HSAAgents[device_id];
 
int32_t __tgt_rtl_init_device(int device_id) {
   }
 
   // Get thread limit
   hsa_dim3_t grid_max_dim;
   err = hsa_agent_get_info(agent, HSA_AGENT_INFO_GRID_MAX_DIM, &grid_max_dim);
   if (err == HSA_STATUS_SUCCESS) {
     DeviceInfo.ThreadsPerGroup[device_id] =
         reinterpret_cast<uint32_t *>(&grid_max_dim)[0] /
         DeviceInfo.GroupsPerDevice[device_id];
-    if ((DeviceInfo.ThreadsPerGroup[device_id] >
-         RTLDeviceInfoTy::Max_WG_Size) ||
-        DeviceInfo.ThreadsPerGroup[device_id] == 0) {
-      DP("Capped thread limit: %d\n", RTLDeviceInfoTy::Max_WG_Size);
+
+    if (DeviceInfo.ThreadsPerGroup[device_id] == 0) {
       DeviceInfo.ThreadsPerGroup[device_id] = RTLDeviceInfoTy::Max_WG_Size;
+      DP("Default thread limit: %d\n", RTLDeviceInfoTy::Max_WG_Size);
+    } else if (enforce_upper_bound(&DeviceInfo.ThreadsPerGroup[device_id],
+                                   RTLDeviceInfoTy::Max_WG_Size)) {
+      DP("Capped thread limit: %d\n", RTLDeviceInfoTy::Max_WG_Size);
     } else {
       DP("Using ROCm Queried thread limit: %d\n",
          DeviceInfo.ThreadsPerGroup[device_id]);
     }
   } else {
     DeviceInfo.ThreadsPerGroup[device_id] = RTLDeviceInfoTy::Max_WG_Size;
     DP("Error getting max block dimension, use default:%d \n",
        RTLDeviceInfoTy::Max_WG_Size);
int32_t __tgt_rtl_init_device(int device_id) {
   } else {
     DP("Default wavefront size: %d\n",
        llvm::omp::AMDGPUGpuGridValues[llvm::omp::GVIDX::GV_Warp_Size]);
     DeviceInfo.WarpSize[device_id] =
         llvm::omp::AMDGPUGpuGridValues[llvm::omp::GVIDX::GV_Warp_Size];
   }
 
   // Adjust teams to the env variables
+
   if (DeviceInfo.EnvTeamLimit > 0 &&
-      DeviceInfo.GroupsPerDevice[device_id] > DeviceInfo.EnvTeamLimit) {
-    DeviceInfo.GroupsPerDevice[device_id] = DeviceInfo.EnvTeamLimit;
+      (enforce_upper_bound(&DeviceInfo.GroupsPerDevice[device_id],
+                           DeviceInfo.EnvTeamLimit))) {
     DP("Capping max groups per device to OMP_TEAM_LIMIT=%d\n",
        DeviceInfo.EnvTeamLimit);
   }
 
   // Set default number of teams
   if (DeviceInfo.EnvNumTeams > 0) {
     DeviceInfo.NumTeams[device_id] = DeviceInfo.EnvNumTeams;
     DP("Default number of teams set according to environment %d\n",
        DeviceInfo.EnvNumTeams);
   } else {
     char *TeamsPerCUEnvStr = getenv("OMP_TARGET_TEAMS_PER_PROC");
     int TeamsPerCU = 1; // default number of teams per CU is 1
     if (TeamsPerCUEnvStr) {
       TeamsPerCU = std::stoi(TeamsPerCUEnvStr);
     }
 
     DeviceInfo.NumTeams[device_id] =
         TeamsPerCU * DeviceInfo.ComputeUnits[device_id];
     DP("Default number of teams = %d * number of compute units %d\n",
        TeamsPerCU, DeviceInfo.ComputeUnits[device_id]);
   }
 
-  if (DeviceInfo.NumTeams[device_id] > DeviceInfo.GroupsPerDevice[device_id]) {
-    DeviceInfo.NumTeams[device_id] = DeviceInfo.GroupsPerDevice[device_id];
+  if (enforce_upper_bound(&DeviceInfo.NumTeams[device_id],
+                          DeviceInfo.GroupsPerDevice[device_id])) {
     DP("Default number of teams exceeds device limit, capping at %d\n",
        DeviceInfo.GroupsPerDevice[device_id]);
   }
 
   // Set default number of threads
   DeviceInfo.NumThreads[device_id] = RTLDeviceInfoTy::Default_WG_Size;
   DP("Default number of threads set according to library's default %d\n",
      RTLDeviceInfoTy::Default_WG_Size);
-  if (DeviceInfo.NumThreads[device_id] >
-      DeviceInfo.ThreadsPerGroup[device_id]) {
-    DeviceInfo.NumThreads[device_id] = DeviceInfo.ThreadsPerGroup[device_id];
+  if (enforce_upper_bound(&DeviceInfo.NumThreads[device_id],
+                          DeviceInfo.ThreadsPerGroup[device_id])) {
     DP("Default number of threads exceeds device limit, capping at %d\n",
        DeviceInfo.ThreadsPerGroup[device_id]);
   }
 
   DP("Device %d: default limit for groupsPerDevice %d & threadsPerGroup %d\n",
      device_id, DeviceInfo.GroupsPerDevice[device_id],
      DeviceInfo.ThreadsPerGroup[device_id]);