[Openmp-commits] [openmp] Revert "[Libomptarget] Rework Record & Replay to be a plugin member" (PR #89028)

Jan Patrick Lehr via Openmp-commits openmp-commits at lists.llvm.org
Tue Apr 16 23:55:21 PDT 2024


https://github.com/jplehr created https://github.com/llvm/llvm-project/pull/89028

Reverts llvm/llvm-project#88928

This broke the AMDGPU buildbots:
https://lab.llvm.org/buildbot/#/builders/193/builds/50201 
https://lab.llvm.org/staging/#/builders/185/builds/5565
https://lab.llvm.org/buildbot/#/builders/259/builds/2955

>From 62305b2e66879cefebf25af1720963f81903ca01 Mon Sep 17 00:00:00 2001
From: Jan Patrick Lehr <JanPatrick.Lehr at amd.com>
Date: Wed, 17 Apr 2024 08:54:04 +0200
Subject: [PATCH] Revert "[Libomptarget] Rework Record & Replay to be a plugin
 member (#88928)"

This reverts commit 9a0a28f8384b2cb534953df33bf124f01f0e0d0e.
---
 .../common/include/PluginInterface.h          | 11 ------
 .../common/src/PluginInterface.cpp            | 39 +++++++------------
 2 files changed, 15 insertions(+), 35 deletions(-)

diff --git a/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h b/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h
index 7f05464f36c1f3..79e8464bfda5c1 100644
--- a/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h
+++ b/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h
@@ -45,8 +45,6 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/Triple.h"
 
-struct RecordReplayTy;
-
 namespace llvm {
 namespace omp {
 namespace target {
@@ -1033,12 +1031,6 @@ struct GenericPluginTy {
     return *RPCServer;
   }
 
-  /// Get a reference to the R&R interface for this plugin.
-  RecordReplayTy &getRecordAndReplay() const {
-    assert(RecordReplay && "R&R not initialized");
-    return *RecordReplay;
-  }
-
   /// Get the OpenMP requires flags set for this plugin.
   int64_t getRequiresFlags() const { return RequiresFlags; }
 
@@ -1228,9 +1220,6 @@ struct GenericPluginTy {
 
   /// The interface between the plugin and the GPU for host services.
   RPCServerTy *RPCServer;
-
-  /// The interface into the record-and-replay functionality.
-  RecordReplayTy *RecordReplay;
 };
 
 namespace Plugin {
diff --git a/openmp/libomptarget/plugins-nextgen/common/src/PluginInterface.cpp b/openmp/libomptarget/plugins-nextgen/common/src/PluginInterface.cpp
index 6df9798f12e3d0..b5f3c45c835fdb 100644
--- a/openmp/libomptarget/plugins-nextgen/common/src/PluginInterface.cpp
+++ b/openmp/libomptarget/plugins-nextgen/common/src/PluginInterface.cpp
@@ -362,6 +362,8 @@ struct RecordReplayTy {
   }
 };
 
+static RecordReplayTy RecordReplay;
+
 // Extract the mapping of host function pointers to device function pointers
 // from the entry table. Functions marked as 'indirect' in OpenMP will have
 // offloading entries generated for them which map the host's function pointer
@@ -471,8 +473,7 @@ GenericKernelTy::getKernelLaunchEnvironment(
   // Ctor/Dtor have no arguments, replaying uses the original kernel launch
   // environment. Older versions of the compiler do not generate a kernel
   // launch environment.
-  if (isCtorOrDtor() ||
-      GenericDevice.Plugin.getRecordAndReplay().isReplaying() ||
+  if (isCtorOrDtor() || RecordReplay.isReplaying() ||
       Version < OMP_KERNEL_ARG_MIN_VERSION_WITH_DYN_PTR)
     return nullptr;
 
@@ -561,7 +562,6 @@ Error GenericKernelTy::launch(GenericDeviceTy &GenericDevice, void **ArgPtrs,
 
   // Record the kernel description after we modified the argument count and num
   // blocks/threads.
-  RecordReplayTy &RecordReplay = GenericDevice.Plugin.getRecordAndReplay();
   if (RecordReplay.isRecording()) {
     RecordReplay.saveImage(getName(), getImage());
     RecordReplay.saveKernelInput(getName(), getImage());
@@ -839,6 +839,9 @@ Error GenericDeviceTy::deinit(GenericPluginTy &Plugin) {
     delete MemoryManager;
   MemoryManager = nullptr;
 
+  if (RecordReplay.isRecordingOrReplaying())
+    RecordReplay.deinit();
+
   if (RPCServer)
     if (auto Err = RPCServer->deinitDevice(*this))
       return Err;
@@ -855,7 +858,6 @@ Error GenericDeviceTy::deinit(GenericPluginTy &Plugin) {
 
   return deinitImpl();
 }
-
 Expected<DeviceImageTy *>
 GenericDeviceTy::loadBinary(GenericPluginTy &Plugin,
                             const __tgt_device_image *InputTgtImage) {
@@ -890,8 +892,7 @@ GenericDeviceTy::loadBinary(GenericPluginTy &Plugin,
     return std::move(Err);
 
   // Setup the global device memory pool if needed.
-  if (!Plugin.getRecordAndReplay().isReplaying() &&
-      shouldSetupDeviceMemoryPool()) {
+  if (!RecordReplay.isReplaying() && shouldSetupDeviceMemoryPool()) {
     uint64_t HeapSize;
     auto SizeOrErr = getDeviceHeapSize(HeapSize);
     if (SizeOrErr) {
@@ -1306,8 +1307,8 @@ Expected<void *> GenericDeviceTy::dataAlloc(int64_t Size, void *HostPtr,
                                             TargetAllocTy Kind) {
   void *Alloc = nullptr;
 
-  if (Plugin.getRecordAndReplay().isRecordingOrReplaying())
-    return Plugin.getRecordAndReplay().alloc(Size);
+  if (RecordReplay.isRecordingOrReplaying())
+    return RecordReplay.alloc(Size);
 
   switch (Kind) {
   case TARGET_ALLOC_DEFAULT:
@@ -1343,7 +1344,7 @@ Expected<void *> GenericDeviceTy::dataAlloc(int64_t Size, void *HostPtr,
 
 Error GenericDeviceTy::dataDelete(void *TgtPtr, TargetAllocTy Kind) {
   // Free is a noop when recording or replaying.
-  if (Plugin.getRecordAndReplay().isRecordingOrReplaying())
+  if (RecordReplay.isRecordingOrReplaying())
     return Plugin::success();
 
   int Res;
@@ -1395,7 +1396,6 @@ Error GenericDeviceTy::launchKernel(void *EntryPtr, void **ArgPtrs,
                                     ptrdiff_t *ArgOffsets,
                                     KernelArgsTy &KernelArgs,
                                     __tgt_async_info *AsyncInfo) {
-  RecordReplayTy &RecordReplay = Plugin.getRecordAndReplay();
   AsyncInfoWrapperTy AsyncInfoWrapper(
       *this, RecordReplay.isRecordingOrReplaying() ? nullptr : AsyncInfo);
 
@@ -1495,9 +1495,6 @@ Error GenericPluginTy::init() {
   RPCServer = new RPCServerTy(*this);
   assert(RPCServer && "Invalid RPC server");
 
-  RecordReplay = new RecordReplayTy();
-  assert(RecordReplay && "Invalid Record and Replay handler");
-
   return Plugin::success();
 }
 
@@ -1511,9 +1508,6 @@ Error GenericPluginTy::deinit() {
     assert(!Devices[DeviceId] && "Device was not deinitialized");
   }
 
-  if (RecordReplay && RecordReplay->isRecordingOrReplaying())
-    RecordReplay->deinit();
-
   // There is no global handler if no device is available.
   if (GlobalHandler)
     delete GlobalHandler;
@@ -1521,9 +1515,6 @@ Error GenericPluginTy::deinit() {
   if (RPCServer)
     delete RPCServer;
 
-  if (RecordReplay)
-    delete RecordReplay;
-
   // Perform last deinitializations on the plugin.
   return deinitImpl();
 }
@@ -1639,12 +1630,12 @@ int32_t GenericPluginTy::initialize_record_replay(int32_t DeviceId,
       isRecord ? RecordReplayTy::RRStatusTy::RRRecording
                : RecordReplayTy::RRStatusTy::RRReplaying;
 
-  if (auto Err = RecordReplay->init(&Device, MemorySize, VAddr, Status,
-                                    SaveOutput, ReqPtrArgOffset)) {
+  if (auto Err = RecordReplay.init(&Device, MemorySize, VAddr, Status,
+                                   SaveOutput, ReqPtrArgOffset)) {
     REPORT("WARNING RR did not intialize RR-properly with %lu bytes"
            "(Error: %s)\n",
            MemorySize, toString(std::move(Err)).data());
-    RecordReplay->setStatus(RecordReplayTy::RRStatusTy::RRDeactivated);
+    RecordReplay.setStatus(RecordReplayTy::RRStatusTy::RRDeactivated);
 
     if (!isRecord) {
       return OFFLOAD_FAIL;
@@ -1993,8 +1984,8 @@ int32_t GenericPluginTy::get_global(__tgt_device_binary Binary, uint64_t Size,
   assert(DevicePtr && "Invalid device global's address");
 
   // Save the loaded globals if we are recording.
-  if (getRecordAndReplay().isRecording())
-    getRecordAndReplay().addEntry(Name, Size, *DevicePtr);
+  if (RecordReplay.isRecording())
+    RecordReplay.addEntry(Name, Size, *DevicePtr);
 
   return OFFLOAD_SUCCESS;
 }



More information about the Openmp-commits mailing list