Skip to content

Commit 2cad43c

Browse files
authored
[Libomptarget] Factor functions out of 'Plugin' interface (llvm#86528)
Summary: This patch factors common functions out of the `Plugin` interface prior to its removal in a future patch. This simply temporarily renames it to `PluginTy` so that we could re-use `Plugin::check` internally as this needs to be defined statically per plugin now. We can refactor this later. The future patch will delete `PluginTy` and `PluginTy::get` entirely. This simply tries to minimize a few changes to make it easier to land.
1 parent b761137 commit 2cad43c

File tree

6 files changed

+132
-121
lines changed

6 files changed

+132
-121
lines changed

openmp/libomptarget/CMakeLists.txt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,7 @@ add_subdirectory(DeviceRTL)
145145
add_subdirectory(tools)
146146

147147
# Build target agnostic offloading library.
148-
set(LIBOMPTARGET_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR}/src)
149-
add_subdirectory(${LIBOMPTARGET_SRC_DIR})
148+
add_subdirectory(src)
150149

151150
# Add tests.
152151
add_subdirectory(test)

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

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2088,7 +2088,7 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy {
20882088
/// Allocate and construct an AMDGPU kernel.
20892089
Expected<GenericKernelTy &> constructKernel(const char *Name) override {
20902090
// Allocate and construct the AMDGPU kernel.
2091-
AMDGPUKernelTy *AMDGPUKernel = Plugin::get().allocate<AMDGPUKernelTy>();
2091+
AMDGPUKernelTy *AMDGPUKernel = PluginTy::get().allocate<AMDGPUKernelTy>();
20922092
if (!AMDGPUKernel)
20932093
return Plugin::error("Failed to allocate memory for AMDGPU kernel");
20942094

@@ -2139,7 +2139,7 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy {
21392139
int32_t ImageId) override {
21402140
// Allocate and initialize the image object.
21412141
AMDGPUDeviceImageTy *AMDImage =
2142-
Plugin::get().allocate<AMDGPUDeviceImageTy>();
2142+
PluginTy::get().allocate<AMDGPUDeviceImageTy>();
21432143
new (AMDImage) AMDGPUDeviceImageTy(ImageId, *this, TgtImage);
21442144

21452145
// Load the HSA executable.
@@ -2697,7 +2697,7 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy {
26972697
}
26982698
Error setDeviceHeapSize(uint64_t Value) override {
26992699
for (DeviceImageTy *Image : LoadedImages)
2700-
if (auto Err = setupDeviceMemoryPool(Plugin::get(), *Image, Value))
2700+
if (auto Err = setupDeviceMemoryPool(PluginTy::get(), *Image, Value))
27012701
return Err;
27022702
DeviceMemoryPoolSize = Value;
27032703
return Plugin::success();
@@ -2737,7 +2737,7 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy {
27372737
return utils::iterateAgentMemoryPools(
27382738
Agent, [&](hsa_amd_memory_pool_t HSAMemoryPool) {
27392739
AMDGPUMemoryPoolTy *MemoryPool =
2740-
Plugin::get().allocate<AMDGPUMemoryPoolTy>();
2740+
PluginTy::get().allocate<AMDGPUMemoryPoolTy>();
27412741
new (MemoryPool) AMDGPUMemoryPoolTy(HSAMemoryPool);
27422742
AllMemoryPools.push_back(MemoryPool);
27432743
return HSA_STATUS_SUCCESS;
@@ -3115,6 +3115,17 @@ struct AMDGPUPluginTy final : public GenericPluginTy {
31153115
return Plugin::check(Status, "Error in hsa_shut_down: %s");
31163116
}
31173117

3118+
/// Creates an AMDGPU device.
3119+
GenericDeviceTy *createDevice(int32_t DeviceId, int32_t NumDevices) override {
3120+
return new AMDGPUDeviceTy(DeviceId, NumDevices, getHostDevice(),
3121+
getKernelAgent(DeviceId));
3122+
}
3123+
3124+
/// Creates an AMDGPU global handler.
3125+
GenericGlobalHandlerTy *createGlobalHandler() override {
3126+
return new AMDGPUGlobalHandlerTy();
3127+
}
3128+
31183129
Triple::ArchType getTripleArch() const override { return Triple::amdgcn; }
31193130

31203131
/// Get the ELF code for recognizing the compatible image binary.
@@ -3237,7 +3248,7 @@ Error AMDGPUKernelTy::launchImpl(GenericDeviceTy &GenericDevice,
32373248
// 56 bytes per allocation.
32383249
uint32_t AllArgsSize = KernelArgsSize + ImplicitArgsSize;
32393250

3240-
AMDHostDeviceTy &HostDevice = Plugin::get<AMDGPUPluginTy>().getHostDevice();
3251+
AMDHostDeviceTy &HostDevice = PluginTy::get<AMDGPUPluginTy>().getHostDevice();
32413252
AMDGPUMemoryManagerTy &ArgsMemoryManager = HostDevice.getArgsMemoryManager();
32423253

32433254
void *AllArgs = nullptr;
@@ -3347,20 +3358,10 @@ Error AMDGPUKernelTy::printLaunchInfoDetails(GenericDeviceTy &GenericDevice,
33473358
return Plugin::success();
33483359
}
33493360

3350-
GenericPluginTy *Plugin::createPlugin() { return new AMDGPUPluginTy(); }
3351-
3352-
GenericDeviceTy *Plugin::createDevice(int32_t DeviceId, int32_t NumDevices) {
3353-
AMDGPUPluginTy &Plugin = get<AMDGPUPluginTy &>();
3354-
return new AMDGPUDeviceTy(DeviceId, NumDevices, Plugin.getHostDevice(),
3355-
Plugin.getKernelAgent(DeviceId));
3356-
}
3357-
3358-
GenericGlobalHandlerTy *Plugin::createGlobalHandler() {
3359-
return new AMDGPUGlobalHandlerTy();
3360-
}
3361+
GenericPluginTy *PluginTy::createPlugin() { return new AMDGPUPluginTy(); }
33613362

33623363
template <typename... ArgsTy>
3363-
Error Plugin::check(int32_t Code, const char *ErrFmt, ArgsTy... Args) {
3364+
static Error Plugin::check(int32_t Code, const char *ErrFmt, ArgsTy... Args) {
33643365
hsa_status_t ResultCode = static_cast<hsa_status_t>(Code);
33653366
if (ResultCode == HSA_STATUS_SUCCESS || ResultCode == HSA_STATUS_INFO_BREAK)
33663367
return Error::success();
@@ -3384,7 +3385,7 @@ void *AMDGPUMemoryManagerTy::allocate(size_t Size, void *HstPtr,
33843385
}
33853386
assert(Ptr && "Invalid pointer");
33863387

3387-
auto &KernelAgents = Plugin::get<AMDGPUPluginTy>().getKernelAgents();
3388+
auto &KernelAgents = PluginTy::get<AMDGPUPluginTy>().getKernelAgents();
33883389

33893390
// Allow all kernel agents to access the allocation.
33903391
if (auto Err = MemoryPool->enableAccess(Ptr, Size, KernelAgents)) {
@@ -3427,7 +3428,7 @@ void *AMDGPUDeviceTy::allocate(size_t Size, void *, TargetAllocTy Kind) {
34273428
}
34283429

34293430
if (Alloc) {
3430-
auto &KernelAgents = Plugin::get<AMDGPUPluginTy>().getKernelAgents();
3431+
auto &KernelAgents = PluginTy::get<AMDGPUPluginTy>().getKernelAgents();
34313432
// Inherently necessary for host or shared allocations
34323433
// Also enabled for device memory to allow device to device memcpy
34333434

openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -976,6 +976,13 @@ struct GenericPluginTy {
976976
Error deinit();
977977
virtual Error deinitImpl() = 0;
978978

979+
/// Create a new device for the underlying plugin.
980+
virtual GenericDeviceTy *createDevice(int32_t DeviceID,
981+
int32_t NumDevices) = 0;
982+
983+
/// Create a new global handler for the underlying plugin.
984+
virtual GenericGlobalHandlerTy *createGlobalHandler() = 0;
985+
979986
/// Get the reference to the device with a certain device id.
980987
GenericDeviceTy &getDevice(int32_t DeviceId) {
981988
assert(isValidDeviceId(DeviceId) && "Invalid device id");
@@ -1085,29 +1092,53 @@ struct GenericPluginTy {
10851092
RPCServerTy *RPCServer;
10861093
};
10871094

1095+
namespace Plugin {
1096+
/// Create a success error. This is the same as calling Error::success(), but
1097+
/// it is recommended to use this one for consistency with Plugin::error() and
1098+
/// Plugin::check().
1099+
static Error success() { return Error::success(); }
1100+
1101+
/// Create a string error.
1102+
template <typename... ArgsTy>
1103+
static Error error(const char *ErrFmt, ArgsTy... Args) {
1104+
return createStringError(inconvertibleErrorCode(), ErrFmt, Args...);
1105+
}
1106+
1107+
/// Check the plugin-specific error code and return an error or success
1108+
/// accordingly. In case of an error, create a string error with the error
1109+
/// description. The ErrFmt should follow the format:
1110+
/// "Error in <function name>[<optional info>]: %s"
1111+
/// The last format specifier "%s" is mandatory and will be used to place the
1112+
/// error code's description. Notice this function should be only called from
1113+
/// the plugin-specific code.
1114+
/// TODO: Refactor this, must be defined individually by each plugin.
1115+
template <typename... ArgsTy>
1116+
static Error check(int32_t ErrorCode, const char *ErrFmt, ArgsTy... Args);
1117+
} // namespace Plugin
1118+
10881119
/// Class for simplifying the getter operation of the plugin. Anywhere on the
10891120
/// code, the current plugin can be retrieved by Plugin::get(). The class also
10901121
/// declares functions to create plugin-specific object instances. The check(),
10911122
/// createPlugin(), createDevice() and createGlobalHandler() functions should be
10921123
/// defined by each plugin implementation.
1093-
class Plugin {
1124+
class PluginTy {
10941125
// Reference to the plugin instance.
10951126
static GenericPluginTy *SpecificPlugin;
10961127

1097-
Plugin() {
1128+
PluginTy() {
10981129
if (auto Err = init())
10991130
REPORT("Failed to initialize plugin: %s\n",
11001131
toString(std::move(Err)).data());
11011132
}
11021133

1103-
~Plugin() {
1134+
~PluginTy() {
11041135
if (auto Err = deinit())
11051136
REPORT("Failed to deinitialize plugin: %s\n",
11061137
toString(std::move(Err)).data());
11071138
}
11081139

1109-
Plugin(const Plugin &) = delete;
1110-
void operator=(const Plugin &) = delete;
1140+
PluginTy(const PluginTy &) = delete;
1141+
void operator=(const PluginTy &) = delete;
11111142

11121143
/// Create and intialize the plugin instance.
11131144
static Error init() {
@@ -1158,7 +1189,7 @@ class Plugin {
11581189
// This static variable will initialize the underlying plugin instance in
11591190
// case there was no previous explicit initialization. The initialization is
11601191
// thread safe.
1161-
static Plugin Plugin;
1192+
static PluginTy Plugin;
11621193

11631194
assert(SpecificPlugin && "Plugin is not active");
11641195
return *SpecificPlugin;
@@ -1170,35 +1201,8 @@ class Plugin {
11701201
/// Indicate whether the plugin is active.
11711202
static bool isActive() { return SpecificPlugin != nullptr; }
11721203

1173-
/// Create a success error. This is the same as calling Error::success(), but
1174-
/// it is recommended to use this one for consistency with Plugin::error() and
1175-
/// Plugin::check().
1176-
static Error success() { return Error::success(); }
1177-
1178-
/// Create a string error.
1179-
template <typename... ArgsTy>
1180-
static Error error(const char *ErrFmt, ArgsTy... Args) {
1181-
return createStringError(inconvertibleErrorCode(), ErrFmt, Args...);
1182-
}
1183-
1184-
/// Check the plugin-specific error code and return an error or success
1185-
/// accordingly. In case of an error, create a string error with the error
1186-
/// description. The ErrFmt should follow the format:
1187-
/// "Error in <function name>[<optional info>]: %s"
1188-
/// The last format specifier "%s" is mandatory and will be used to place the
1189-
/// error code's description. Notice this function should be only called from
1190-
/// the plugin-specific code.
1191-
template <typename... ArgsTy>
1192-
static Error check(int32_t ErrorCode, const char *ErrFmt, ArgsTy... Args);
1193-
11941204
/// Create a plugin instance.
11951205
static GenericPluginTy *createPlugin();
1196-
1197-
/// Create a plugin-specific device.
1198-
static GenericDeviceTy *createDevice(int32_t DeviceId, int32_t NumDevices);
1199-
1200-
/// Create a plugin-specific global handler.
1201-
static GenericGlobalHandlerTy *createGlobalHandler();
12021206
};
12031207

12041208
/// Auxiliary interface class for GenericDeviceResourceManagerTy. This class

0 commit comments

Comments
 (0)