[llvm] [Offload] Ensure all `llvm::Error`s are handled (PR #137339)
Ross Brunton via llvm-commits
llvm-commits at lists.llvm.org
Thu May 1 07:41:38 PDT 2025
https://github.com/RossBrunton updated https://github.com/llvm/llvm-project/pull/137339
>From e498b7937d3c4da33e70b127ba7de39ba0fc5896 Mon Sep 17 00:00:00 2001
From: Ross Brunton <ross at codeplay.com>
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 | 13 ++++++
offload/liboffload/src/OffloadImpl.cpp | 45 ++++++++++++-------
.../OffloadAPI/kernel/olGetKernel.cpp | 7 +++
3 files changed, 50 insertions(+), 15 deletions(-)
diff --git a/offload/liboffload/include/OffloadImpl.hpp b/offload/liboffload/include/OffloadImpl.hpp
index ec470a355309a..c7a41374adc49 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,18 @@ struct ol_impl_result_t {
Result = errors().emplace(std::move(Err)).first->get();
}
+ static ol_impl_result_t fromError(llvm::Error &&Error,
+ llvm::StringRef Details) {
+ ol_errc_t ErrCode;
+ 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;
+ });
+
+ 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..433e2efeff891 100644
--- a/offload/liboffload/src/OffloadImpl.cpp
+++ b/offload/liboffload/src/OffloadImpl.cpp
@@ -311,8 +311,9 @@ 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()};
+ ol_impl_result_t::fromError(
+ Alloc.takeError(),
+ formatv("Could not create allocation on device {0}", Device).str());
*AllocationOut = *Alloc;
allocInfoMap().insert_or_assign(*Alloc, AllocInfo{Device, Type});
@@ -330,7 +331,8 @@ 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),
+ "Could not free allocation");
allocInfoMap().erase(Address);
@@ -342,7 +344,8 @@ ol_impl_result_t olCreateQueue_impl(ol_device_handle_t Device,
auto CreatedQueue = std::make_unique<ol_queue_impl_t>(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),
+ "Could not initialize stream resource");
*Queue = CreatedQueue.release();
return OL_SUCCESS;
@@ -358,7 +361,8 @@ 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"};
+ ol_impl_result_t::fromError(std::move(Err),
+ "The queue failed to synchronize");
}
// Recreate the stream resource so the queue can be reused
@@ -366,7 +370,8 @@ 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), "Could not reinitialize the stream resource");
return OL_SUCCESS;
}
@@ -374,7 +379,8 @@ 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"};
+ ol_impl_result_t::fromError(std::move(Res),
+ "The event failed to synchronize");
return OL_SUCCESS;
}
@@ -390,13 +396,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<ol_event_impl_t>(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 +432,19 @@ 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),
+ "The data retrieve operation failed");
} 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),
+ "The data submit operation failed");
} 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),
+ "The data exchange operation failed");
}
if (EventOut)
@@ -459,7 +472,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(), "Invalid binary");
}
Prog->Image = *Res;
@@ -483,7 +496,8 @@ 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),
+ "Could not initialize the kernel");
*Kernel = &*KernelImpl;
@@ -526,7 +540,8 @@ 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), "Could not finalize the AsyncInfoWrapper");
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));
+}
More information about the llvm-commits
mailing list