From 18d142658f185b1a8d0ca12b919f3dd4fff3442c Mon Sep 17 00:00:00 2001 From: Ross Brunton Date: Fri, 25 Apr 2025 15:18:56 +0100 Subject: [PATCH] [Offload] Ensure all `llvm::Error`s are handled `llvm::Error`s containing errors must be explicitly handled or an assert will be raised. With this change, `ol_impl_result_t` can accept and consume an `llvm::Error` for errors raised by PluginInterface. Note that there is currently no facility for PluginInterface to communicate exact error codes, but the constructor is designed in such a way that it can be easily added later. This MR is to convert a crash into an error code. A new test was added, however due to the aforementioned issue with error codes, it does not pass and instead is disabled. --- offload/liboffload/include/OffloadImpl.hpp | 14 ++++++++ offload/liboffload/src/OffloadImpl.cpp | 33 ++++++++++--------- .../OffloadAPI/kernel/olGetKernel.cpp | 7 ++++ 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/offload/liboffload/include/OffloadImpl.hpp b/offload/liboffload/include/OffloadImpl.hpp index ec470a355309a..48fdc2b884199 100644 --- a/offload/liboffload/include/OffloadImpl.hpp +++ b/offload/liboffload/include/OffloadImpl.hpp @@ -19,6 +19,7 @@ #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" +#include "llvm/Support/Error.h" struct OffloadConfig { bool TracingEnabled = false; @@ -88,6 +89,19 @@ struct ol_impl_result_t { Result = errors().emplace(std::move(Err)).first->get(); } + static ol_impl_result_t fromError(llvm::Error &&Error) { + ol_errc_t ErrCode; + llvm::StringRef Details; + llvm::handleAllErrors(std::move(Error), [&](llvm::StringError &Err) { + // TODO: PluginInterface doesn't yet have a way to communicate offload + // error codes + ErrCode = OL_ERRC_UNKNOWN; + Details = errorStrs().insert(Err.getMessage()).first->getKeyData(); + }); + + return ol_impl_result_t{ErrCode, Details}; + } + operator ol_result_t() { return Result; } private: diff --git a/offload/liboffload/src/OffloadImpl.cpp b/offload/liboffload/src/OffloadImpl.cpp index bef72a7d1851a..b50c7e0f87b7c 100644 --- a/offload/liboffload/src/OffloadImpl.cpp +++ b/offload/liboffload/src/OffloadImpl.cpp @@ -311,8 +311,7 @@ ol_impl_result_t olMemAlloc_impl(ol_device_handle_t Device, auto Alloc = Device->Device->dataAlloc(Size, nullptr, convertOlToPluginAllocTy(Type)); if (!Alloc) - return {OL_ERRC_OUT_OF_RESOURCES, - formatv("Could not create allocation on device {0}", Device).str()}; + return ol_impl_result_t::fromError(Alloc.takeError()); *AllocationOut = *Alloc; allocInfoMap().insert_or_assign(*Alloc, AllocInfo{Device, Type}); @@ -330,7 +329,7 @@ ol_impl_result_t olMemFree_impl(void *Address) { auto Res = Device->Device->dataDelete(Address, convertOlToPluginAllocTy(Type)); if (Res) - return {OL_ERRC_OUT_OF_RESOURCES, "Could not free allocation"}; + return ol_impl_result_t::fromError(std::move(Res)); allocInfoMap().erase(Address); @@ -342,7 +341,7 @@ ol_impl_result_t olCreateQueue_impl(ol_device_handle_t Device, auto CreatedQueue = std::make_unique(nullptr, Device); auto Err = Device->Device->initAsyncInfo(&(CreatedQueue->AsyncInfo)); if (Err) - return {OL_ERRC_UNKNOWN, "Could not initialize stream resource"}; + return ol_impl_result_t::fromError(std::move(Err)); *Queue = CreatedQueue.release(); return OL_SUCCESS; @@ -358,7 +357,7 @@ ol_impl_result_t olWaitQueue_impl(ol_queue_handle_t Queue) { if (Queue->AsyncInfo->Queue) { auto Err = Queue->Device->Device->synchronize(Queue->AsyncInfo); if (Err) - return {OL_ERRC_INVALID_QUEUE, "The queue failed to synchronize"}; + return ol_impl_result_t::fromError(std::move(Err)); } // Recreate the stream resource so the queue can be reused @@ -366,7 +365,7 @@ ol_impl_result_t olWaitQueue_impl(ol_queue_handle_t Queue) { // it to begin with. auto Res = Queue->Device->Device->initAsyncInfo(&Queue->AsyncInfo); if (Res) - return {OL_ERRC_UNKNOWN, "Could not reinitialize the stream resource"}; + return ol_impl_result_t::fromError(std::move(Res)); return OL_SUCCESS; } @@ -374,7 +373,7 @@ ol_impl_result_t olWaitQueue_impl(ol_queue_handle_t Queue) { ol_impl_result_t olWaitEvent_impl(ol_event_handle_t Event) { auto Res = Event->Queue->Device->Device->syncEvent(Event->EventInfo); if (Res) - return {OL_ERRC_INVALID_EVENT, "The event failed to synchronize"}; + return ol_impl_result_t::fromError(std::move(Res)); return OL_SUCCESS; } @@ -390,13 +389,17 @@ ol_impl_result_t olDestroyEvent_impl(ol_event_handle_t Event) { ol_event_handle_t makeEvent(ol_queue_handle_t Queue) { auto EventImpl = std::make_unique(nullptr, Queue); auto Res = Queue->Device->Device->createEvent(&EventImpl->EventInfo); - if (Res) + if (Res) { + llvm::consumeError(std::move(Res)); return nullptr; + } Res = Queue->Device->Device->recordEvent(EventImpl->EventInfo, Queue->AsyncInfo); - if (Res) + if (Res) { + llvm::consumeError(std::move(Res)); return nullptr; + } return EventImpl.release(); } @@ -422,16 +425,16 @@ ol_impl_result_t olMemcpy_impl(ol_queue_handle_t Queue, void *DstPtr, if (DstDevice == HostDevice()) { auto Res = SrcDevice->Device->dataRetrieve(DstPtr, SrcPtr, Size, QueueImpl); if (Res) - return {OL_ERRC_UNKNOWN, "The data retrieve operation failed"}; + return ol_impl_result_t::fromError(std::move(Res)); } else if (SrcDevice == HostDevice()) { auto Res = DstDevice->Device->dataSubmit(DstPtr, SrcPtr, Size, QueueImpl); if (Res) - return {OL_ERRC_UNKNOWN, "The data submit operation failed"}; + return ol_impl_result_t::fromError(std::move(Res)); } else { auto Res = SrcDevice->Device->dataExchange(SrcPtr, *DstDevice->Device, DstPtr, Size, QueueImpl); if (Res) - return {OL_ERRC_UNKNOWN, "The data exchange operation failed"}; + return ol_impl_result_t::fromError(std::move(Res)); } if (EventOut) @@ -459,7 +462,7 @@ ol_impl_result_t olCreateProgram_impl(ol_device_handle_t Device, Device->Device->loadBinary(Device->Device->Plugin, &Prog->DeviceImage); if (!Res) { delete Prog; - return OL_ERRC_INVALID_VALUE; + return ol_impl_result_t::fromError(Res.takeError()); } Prog->Image = *Res; @@ -483,7 +486,7 @@ ol_impl_result_t olGetKernel_impl(ol_program_handle_t Program, auto Err = KernelImpl->init(Device, *Program->Image); if (Err) - return {OL_ERRC_UNKNOWN, "Could not initialize the kernel"}; + return ol_impl_result_t::fromError(std::move(Err)); *Kernel = &*KernelImpl; @@ -526,7 +529,7 @@ olLaunchKernel_impl(ol_queue_handle_t Queue, ol_device_handle_t Device, AsyncInfoWrapper.finalize(Err); if (Err) - return {OL_ERRC_UNKNOWN, "Could not finalize the AsyncInfoWrapper"}; + return ol_impl_result_t::fromError(std::move(Err)); if (EventOut) *EventOut = makeEvent(Queue); diff --git a/offload/unittests/OffloadAPI/kernel/olGetKernel.cpp b/offload/unittests/OffloadAPI/kernel/olGetKernel.cpp index 097439e19156b..bd1b562eac71e 100644 --- a/offload/unittests/OffloadAPI/kernel/olGetKernel.cpp +++ b/offload/unittests/OffloadAPI/kernel/olGetKernel.cpp @@ -29,3 +29,10 @@ TEST_P(olGetKernelTest, InvalidNullKernelPointer) { ASSERT_ERROR(OL_ERRC_INVALID_NULL_POINTER, olGetKernel(Program, "foo", nullptr)); } + +// Error code returning from plugin interface not yet supported +TEST_F(olGetKernelTest, DISABLED_InvalidKernelName) { + ol_kernel_handle_t Kernel = nullptr; + ASSERT_ERROR(OL_ERRC_INVALID_KERNEL_NAME, + olGetKernel(Program, "invalid_kernel_name", &Kernel)); +}