Fix review comments

Author: kevinsala

offload/include/omptarget.h

@@ -281,8 +281,8 @@ int omp_get_num_devices(void);
 int omp_get_device_num(void);
 int omp_get_initial_device(void);
 size_t
-omp_get_groupprivate_limit(int device_num,
-                           omp_access_t access_group = omp_access_cgroup);
+omp_get_groupprivate_limit(int DeviceNum,
+                           omp_access_t AccessGroup = omp_access_cgroup);
 void *omp_target_alloc(size_t Size, int DeviceNum);
 void omp_target_free(void *DevicePtr, int DeviceNum);
 int omp_target_is_present(const void *Ptr, int DeviceNum);

offload/liboffload/API/Device.td

@@ -43,6 +43,7 @@ def ol_device_info_t : Enum {
     TaggedEtor<"ADDRESS_BITS", "uint32_t", "Number of bits used to represent an address in device memory">,
     TaggedEtor<"MAX_MEM_ALLOC_SIZE", "uint64_t", "The maximum size of memory object allocation in bytes">,
     TaggedEtor<"GLOBAL_MEM_SIZE", "uint64_t", "The size of global device memory in bytes">,
+    TaggedEtor<"WORK_GROUP_SHARED_MEM_SIZE", "uint64_t", "The maximum size of shared memory per work group in bytes">,
   ];
   list<TaggedEtor> fp_configs = !foreach(type, ["Single", "Double", "Half"], TaggedEtor<type # "_FP_CONFIG", "ol_device_fp_capability_flags_t", type # " precision floating point capability">);
   list<TaggedEtor> native_vec_widths = !foreach(type, ["char","short","int","long","float","double","half"], TaggedEtor<"NATIVE_VECTOR_WIDTH_" # type, "uint32_t", "Native vector width for " # type>);

offload/liboffload/src/OffloadImpl.cpp

@@ -495,6 +495,14 @@ Error olGetDeviceInfoImplDetail(ol_device_handle_t Device,
     return Info.write(static_cast<uint32_t>(Value));
   }
 
+  case OL_DEVICE_INFO_WORK_GROUP_SHARED_MEM_SIZE: {
+    // Uint64 values
+    if (!std::holds_alternative<uint64_t>(Entry->Value))
+      return makeError(ErrorCode::BACKEND_FAILURE,
+                       "plugin returned incorrect type");
+    return Info.write(std::get<uint64_t>(Entry->Value));
+  }
+
   case OL_DEVICE_INFO_MAX_WORK_SIZE_PER_DIMENSION:
   case OL_DEVICE_INFO_MAX_WORK_GROUP_SIZE_PER_DIMENSION: {
     // {x, y, z} triples
@@ -590,6 +598,7 @@ Error olGetDeviceInfoImplDetailHost(ol_device_handle_t Device,
     return Info.write<uint32_t>(std::numeric_limits<uintptr_t>::digits);
   case OL_DEVICE_INFO_MAX_MEM_ALLOC_SIZE:
   case OL_DEVICE_INFO_GLOBAL_MEM_SIZE:
+  case OL_DEVICE_INFO_WORK_GROUP_SHARED_MEM_SIZE:
     return Info.write<uint64_t>(0);
   default:
     return createOffloadError(ErrorCode::INVALID_ENUMERATION,

offload/libomptarget/OpenMP/API.cpp

@@ -83,6 +83,9 @@ EXTERN size_t omp_get_groupprivate_limit(int DeviceNum,
   if (DeviceNum == omp_get_initial_device())
     return 0;
 
+  if (AccessGroup != omp_access_cgroup)
+    return 0;
+
   auto DeviceOrErr = PM->getDevice(DeviceNum);
   if (!DeviceOrErr)
     FATAL_MESSAGE(DeviceNum, "%s", toString(DeviceOrErr.takeError()).c_str());

offload/plugins-nextgen/amdgpu/src/rtl.cpp

@@ -2198,9 +2198,6 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy {
       }
     }
 
-    // Supports block shared memory natively.
-    HasNativeBlockSharedMem = true;
-
     return Plugin::success();
   }
 

offload/plugins-nextgen/common/include/PluginInterface.h

@@ -808,8 +808,8 @@ struct GenericDeviceTy : public DeviceAllocatorTy {
   /// Get the total shared memory per block that can be used in any kernel.
   uint32_t getMaxBlockSharedMemSize() const { return MaxBlockSharedMemSize; }
 
-  /// Indicate whether the device has native block shared memory.
-  bool hasNativeBlockSharedMem() const { return HasNativeBlockSharedMem; }
+  /// Indicate whether the device supports block shared memory natively.
+  bool hasNativeBlockSharedMem() const { return MaxBlockSharedMemSize > 0; }
 
   /// Set the context of the device if needed, before calling device-specific
   /// functions. Plugins may implement this function as a no-op if not needed.
@@ -1269,11 +1269,8 @@ struct GenericDeviceTy : public DeviceAllocatorTy {
   std::atomic<bool> OmptInitialized;
 #endif
 
-  /// The total per-block shared memory that a kernel may use.
+  /// The total per-block native shared memory that a kernel may use.
   uint32_t MaxBlockSharedMemSize = 0;
-
-  /// Whether the device has native block shared memory.
-  bool HasNativeBlockSharedMem = false;
 };
 
 /// Class implementing common functionalities of offload plugins. Each plugin

offload/plugins-nextgen/cuda/src/rtl.cpp

@@ -393,9 +393,6 @@ struct CUDADeviceTy : public GenericDeviceTy {
       return Err;
     MaxBlockSharedMemSize = MaxSharedMem;
 
-    // Supports block shared memory natively.
-    HasNativeBlockSharedMem = true;
-
     return Plugin::success();
   }
 

offload/tools/deviceinfo/llvm-offload-device-info.cpp

@@ -205,6 +205,8 @@ ol_result_t printDevice(std::ostream &S, ol_device_handle_t D) {
       S, D, OL_DEVICE_INFO_MAX_MEM_ALLOC_SIZE, "Max Mem Allocation Size", "B"));
   OFFLOAD_ERR(printDeviceValue<uint64_t>(S, D, OL_DEVICE_INFO_GLOBAL_MEM_SIZE,
                                          "Global Mem Size", "B"));
+  OFFLOAD_ERR(printDeviceValue<uint64_t>(S, D, OL_DEVICE_INFO_WORK_GROUP_SHARED_MEM_SIZE,
+                                         "Work Group Shared Mem Size", "B"));
   OFFLOAD_ERR(
       (printDeviceValue<ol_device_fp_capability_flags_t, PrintKind::FP_FLAGS>(
           S, D, OL_DEVICE_INFO_SINGLE_FP_CONFIG,

offload/unittests/OffloadAPI/device/olGetDeviceInfo.cpp

@@ -217,6 +217,10 @@ OL_DEVICE_INFO_TEST_DEVICE_VALUE_GT(GlobalMemSize, uint64_t,
                                     OL_DEVICE_INFO_GLOBAL_MEM_SIZE, 0);
 OL_DEVICE_INFO_TEST_HOST_SUCCESS(GlobalMemSize, uint64_t,
                                  OL_DEVICE_INFO_GLOBAL_MEM_SIZE);
+OL_DEVICE_INFO_TEST_DEVICE_VALUE_GT(SharedMemSize, uint64_t,
+                                    OL_DEVICE_INFO_WORK_GROUP_SHARED_MEM_SIZE, 0);
+OL_DEVICE_INFO_TEST_HOST_SUCCESS(SharedMemSize, uint64_t,
+                                 OL_DEVICE_INFO_WORK_GROUP_SHARED_MEM_SIZE);
 
 TEST_P(olGetDeviceInfoTest, InvalidNullHandleDevice) {
   ol_device_type_t DeviceType;

offload/unittests/OffloadAPI/device/olGetDeviceInfoSize.cpp

@@ -71,6 +71,8 @@ OL_DEVICE_INFO_SIZE_TEST_EQ(MaxMemAllocSize, uint64_t,
                             OL_DEVICE_INFO_MAX_MEM_ALLOC_SIZE);
 OL_DEVICE_INFO_SIZE_TEST_EQ(GlobalMemSize, uint64_t,
                             OL_DEVICE_INFO_GLOBAL_MEM_SIZE);
+OL_DEVICE_INFO_SIZE_TEST_EQ(SharedMemSize, uint64_t,
+                            OL_DEVICE_INFO_WORK_GROUP_SHARED_MEM_SIZE);
 
 TEST_P(olGetDeviceInfoSizeTest, SuccessMaxWorkGroupSizePerDimension) {
   size_t Size = 0;