Revert "[Offload] Use Error for allocating/deallocating in plugins (llvm#160811)"

This reverts commit 01d761a.

Author: ronlieb

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

@@ -623,11 +623,7 @@ struct AMDGPUMemoryManagerTy : public DeviceAllocatorTy {
     assert(MemoryManager && "Invalid memory manager");
     assert(PtrStorage && "Invalid pointer storage");
 
-    auto PtrStorageOrErr = MemoryManager->allocate(Size, nullptr);
-    if (!PtrStorageOrErr)
-      return PtrStorageOrErr.takeError();
-
-    *PtrStorage = *PtrStorageOrErr;
+    *PtrStorage = MemoryManager->allocate(Size, nullptr);
     if (Size && *PtrStorage == nullptr)
       return Plugin::error(ErrorCode::OUT_OF_RESOURCES,
                            "failure to allocate from AMDGPU memory manager");
@@ -647,12 +643,15 @@ struct AMDGPUMemoryManagerTy : public DeviceAllocatorTy {
 private:
   /// Allocation callback that will be called once the memory manager does not
   /// have more previously allocated buffers.
-  Expected<void *> allocate(size_t Size, void *HstPtr,
-                            TargetAllocTy Kind) override;
+  void *allocate(size_t Size, void *HstPtr, TargetAllocTy Kind) override;
 
   /// Deallocation callback that will be called by the memory manager.
-  Error free(void *TgtPtr, TargetAllocTy Kind) override {
-    return MemoryPool->deallocate(TgtPtr);
+  int free(void *TgtPtr, TargetAllocTy Kind) override {
+    if (auto Err = MemoryPool->deallocate(TgtPtr)) {
+      consumeError(std::move(Err));
+      return OFFLOAD_FAIL;
+    }
+    return OFFLOAD_SUCCESS;
   }
 
   /// The underlying plugin that owns this memory manager.
@@ -3624,12 +3623,12 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy {
   }
 
   /// Allocate memory on the device or related to the device.
-  Expected<void *> allocate(size_t Size, void *, TargetAllocTy Kind) override;
+  void *allocate(size_t Size, void *, TargetAllocTy Kind) override;
 
   /// Deallocate memory on the device or related to the device.
-  Error free(void *TgtPtr, TargetAllocTy Kind) override {
+  int free(void *TgtPtr, TargetAllocTy Kind) override {
     if (TgtPtr == nullptr)
-      return Plugin::success();
+      return OFFLOAD_SUCCESS;
 
     AMDGPUMemoryPoolTy *MemoryPool = nullptr;
     switch (Kind) {
@@ -3645,14 +3644,17 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy {
       break;
     }
 
-    if (!MemoryPool)
-      return Plugin::error(ErrorCode::OUT_OF_RESOURCES,
-                           "no memory pool for the specified allocation kind");
+    if (!MemoryPool) {
+      REPORT("No memory pool for the specified allocation kind\n");
+      return OFFLOAD_FAIL;
+    }
 
-    if (auto Err = MemoryPool->deallocate(TgtPtr))
-      return Err;
+    if (Error Err = MemoryPool->deallocate(TgtPtr)) {
+      REPORT("%s\n", toString(std::move(Err)).data());
+      return OFFLOAD_FAIL;
+    }
 
-    return Plugin::success();
+    return OFFLOAD_SUCCESS;
   }
 
   /// Synchronize current thread with the pending operations on the async info.
@@ -5739,13 +5741,14 @@ static Error Plugin::check(int32_t Code, const char *ErrFmt, ArgsTy... Args) {
   return Plugin::error(OffloadErrCode, ErrFmt, Args..., Desc);
 }
 
-Expected<void *> AMDGPUMemoryManagerTy::allocate(size_t Size, void *HstPtr,
-                                                 TargetAllocTy Kind) {
+void *AMDGPUMemoryManagerTy::allocate(size_t Size, void *HstPtr,
+                                      TargetAllocTy Kind) {
   // Allocate memory from the pool.
   void *Ptr = nullptr;
-  if (auto Err = MemoryPool->allocate(Size, &Ptr))
-    return std::move(Err);
-
+  if (auto Err = MemoryPool->allocate(Size, &Ptr)) {
+    consumeError(std::move(Err));
+    return nullptr;
+  }
   assert(Ptr && "Invalid pointer");
 
   // Get a list of agents that can access this memory pool.
@@ -5755,13 +5758,14 @@ Expected<void *> AMDGPUMemoryManagerTy::allocate(size_t Size, void *HstPtr,
       [&](hsa_agent_t Agent) { return MemoryPool->canAccess(Agent); });
 
   // Allow all valid kernel agents to access the allocation.
-  if (auto Err = MemoryPool->enableAccess(Ptr, Size, Agents))
-    return std::move(Err);
+  if (auto Err = MemoryPool->enableAccess(Ptr, Size, Agents)) {
+    REPORT("%s\n", toString(std::move(Err)).data());
+    return nullptr;
+  }
   return Ptr;
 }
 
-Expected<void *> AMDGPUDeviceTy::allocate(size_t Size, void *,
-                                          TargetAllocTy Kind) {
+void *AMDGPUDeviceTy::allocate(size_t Size, void *, TargetAllocTy Kind) {
   if (Size == 0)
     return nullptr;
 
@@ -5780,14 +5784,17 @@ Expected<void *> AMDGPUDeviceTy::allocate(size_t Size, void *,
     break;
   }
 
-  if (!MemoryPool)
-    return Plugin::error(ErrorCode::UNSUPPORTED,
-                         "no memory pool for the specified allocation kind");
+  if (!MemoryPool) {
+    REPORT("No memory pool for the specified allocation kind\n");
+    return nullptr;
+  }
 
   // Allocate from the corresponding memory pool.
   void *Alloc = nullptr;
-  if (auto Err = MemoryPool->allocate(Size, &Alloc))
-    return std::move(Err);
+  if (Error Err = MemoryPool->allocate(Size, &Alloc)) {
+    REPORT("%s\n", toString(std::move(Err)).data());
+    return nullptr;
+  }
 
   if (Alloc && (Kind == TARGET_ALLOC_HOST || Kind == TARGET_ALLOC_SHARED ||
                 OMPX_EnableDevice2DeviceMemAccess)) {
@@ -5801,8 +5808,10 @@ Expected<void *> AMDGPUDeviceTy::allocate(size_t Size, void *,
                   });
 
     // Enable all valid kernel agents to access the buffer.
-    if (auto Err = MemoryPool->enableAccess(Alloc, Size, Agents))
-      return std::move(Err);
+    if (auto Err = MemoryPool->enableAccess(Alloc, Size, Agents)) {
+      REPORT("%s\n", toString(std::move(Err)).data());
+      return nullptr;
+    }
   }
 
   return Alloc;

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

@@ -25,24 +25,18 @@
 #include "Shared/Utils.h"
 #include "omptarget.h"
 
-#include "llvm/Support/Error.h"
-
-namespace llvm {
-
 /// Base class of per-device allocator.
 class DeviceAllocatorTy {
 public:
   virtual ~DeviceAllocatorTy() = default;
 
   /// Allocate a memory of size \p Size . \p HstPtr is used to assist the
   /// allocation.
-  virtual Expected<void *>
-  allocate(size_t Size, void *HstPtr,
-           TargetAllocTy Kind = TARGET_ALLOC_DEFAULT) = 0;
+  virtual void *allocate(size_t Size, void *HstPtr,
+                         TargetAllocTy Kind = TARGET_ALLOC_DEFAULT) = 0;
 
   /// Delete the pointer \p TgtPtr on the device
-  virtual Error free(void *TgtPtr,
-                     TargetAllocTy Kind = TARGET_ALLOC_DEFAULT) = 0;
+  virtual int free(void *TgtPtr, TargetAllocTy Kind = TARGET_ALLOC_DEFAULT) = 0;
 };
 
 /// Class of memory manager. The memory manager is per-device by using
@@ -140,17 +134,17 @@ class MemoryManagerTy {
   size_t SizeThreshold = 1U << 13;
 
   /// Request memory from target device
-  Expected<void *> allocateOnDevice(size_t Size, void *HstPtr) const {
+  void *allocateOnDevice(size_t Size, void *HstPtr) const {
     return DeviceAllocator.allocate(Size, HstPtr, TARGET_ALLOC_DEVICE);
   }
 
   /// Deallocate data on device
-  Error deleteOnDevice(void *Ptr) const { return DeviceAllocator.free(Ptr); }
+  int deleteOnDevice(void *Ptr) const { return DeviceAllocator.free(Ptr); }
 
   /// This function is called when it tries to allocate memory on device but the
   /// device returns out of memory. It will first free all memory in the
   /// FreeList and try to allocate again.
-  Expected<void *> freeAndAllocate(size_t Size, void *HstPtr) {
+  void *freeAndAllocate(size_t Size, void *HstPtr) {
     std::vector<void *> RemoveList;
 
     // Deallocate all memory in FreeList
@@ -160,8 +154,7 @@ class MemoryManagerTy {
       if (List.empty())
         continue;
       for (const NodeTy &N : List) {
-        if (auto Err = deleteOnDevice(N.Ptr))
-          return Err;
+        deleteOnDevice(N.Ptr);
         RemoveList.push_back(N.Ptr);
       }
       FreeLists[I].clear();
@@ -182,22 +175,14 @@ class MemoryManagerTy {
   /// allocate directly on the device. If a \p nullptr is returned, it might
   /// be because the device is OOM. In that case, it will free all unused
   /// memory and then try again.
-  Expected<void *> allocateOrFreeAndAllocateOnDevice(size_t Size,
-                                                     void *HstPtr) {
-    auto TgtPtrOrErr = allocateOnDevice(Size, HstPtr);
-    if (!TgtPtrOrErr)
-      return TgtPtrOrErr.takeError();
-
-    void *TgtPtr = *TgtPtrOrErr;
+  void *allocateOrFreeAndAllocateOnDevice(size_t Size, void *HstPtr) {
+    void *TgtPtr = allocateOnDevice(Size, HstPtr);
     // We cannot get memory from the device. It might be due to OOM. Let's
     // free all memory in FreeLists and try again.
     if (TgtPtr == nullptr) {
       DP("Failed to get memory on device. Free all memory in FreeLists and "
          "try again.\n");
-      TgtPtrOrErr = freeAndAllocate(Size, HstPtr);
-      if (!TgtPtrOrErr)
-        return TgtPtrOrErr.takeError();
-      TgtPtr = *TgtPtrOrErr;
+      TgtPtr = freeAndAllocate(Size, HstPtr);
     }
 
     if (TgtPtr == nullptr)
@@ -219,17 +204,16 @@ class MemoryManagerTy {
 
   /// Destructor
   ~MemoryManagerTy() {
-    for (auto &PtrToNode : PtrToNodeTable) {
-      assert(PtrToNode.second.Ptr && "nullptr in map table");
-      if (auto Err = deleteOnDevice(PtrToNode.second.Ptr))
-        REPORT("Failure to delete memory: %s\n",
-               toString(std::move(Err)).data());
+    for (auto Itr = PtrToNodeTable.begin(); Itr != PtrToNodeTable.end();
+         ++Itr) {
+      assert(Itr->second.Ptr && "nullptr in map table");
+      deleteOnDevice(Itr->second.Ptr);
     }
   }
 
   /// Allocate memory of size \p Size from target device. \p HstPtr is used to
   /// assist the allocation.
-  Expected<void *> allocate(size_t Size, void *HstPtr) {
+  void *allocate(size_t Size, void *HstPtr) {
     // If the size is zero, we will not bother the target device. Just return
     // nullptr directly.
     if (Size == 0)
@@ -244,14 +228,11 @@ class MemoryManagerTy {
       DP("%zu is greater than the threshold %zu. Allocate it directly from "
          "device\n",
          Size, SizeThreshold);
-      auto TgtPtrOrErr = allocateOrFreeAndAllocateOnDevice(Size, HstPtr);
-      if (!TgtPtrOrErr)
-        return TgtPtrOrErr.takeError();
+      void *TgtPtr = allocateOrFreeAndAllocateOnDevice(Size, HstPtr);
 
-      DP("Got target pointer " DPxMOD ". Return directly.\n",
-         DPxPTR(*TgtPtrOrErr));
+      DP("Got target pointer " DPxMOD ". Return directly.\n", DPxPTR(TgtPtr));
 
-      return *TgtPtrOrErr;
+      return TgtPtr;
     }
 
     NodeTy *NodePtr = nullptr;
@@ -279,11 +260,8 @@ class MemoryManagerTy {
     if (NodePtr == nullptr) {
       DP("Cannot find a node in the FreeLists. Allocate on device.\n");
       // Allocate one on device
-      auto TgtPtrOrErr = allocateOrFreeAndAllocateOnDevice(Size, HstPtr);
-      if (!TgtPtrOrErr)
-        return TgtPtrOrErr.takeError();
+      void *TgtPtr = allocateOrFreeAndAllocateOnDevice(Size, HstPtr);
 
-      void *TgtPtr = *TgtPtrOrErr;
       if (TgtPtr == nullptr)
         return nullptr;
 
@@ -304,7 +282,7 @@ class MemoryManagerTy {
   }
 
   /// Deallocate memory pointed by \p TgtPtr
-  Error free(void *TgtPtr) {
+  int free(void *TgtPtr) {
     DP("MemoryManagerTy::free: target memory " DPxMOD ".\n", DPxPTR(TgtPtr));
 
     NodeTy *P = nullptr;
@@ -336,7 +314,7 @@ class MemoryManagerTy {
       FreeLists[B].insert(*P);
     }
 
-    return Error::success();
+    return OFFLOAD_SUCCESS;
   }
 
   /// Get the size threshold from the environment variable
@@ -366,6 +344,4 @@ class MemoryManagerTy {
 constexpr const size_t MemoryManagerTy::BucketSize[];
 constexpr const int MemoryManagerTy::NumBuckets;
 
-} // namespace llvm
-
 #endif // LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_MEMORYMANAGER_H