Skip to content

Commit 07ebde9

Browse files
RossBruntonIanWood1
authored andcommitted
[Offload] Ensure all llvm::Errors are handled (llvm#137339)
`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 that have multiple causes and other places now call `llvm::consumeError`. 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 marked as a skip.
1 parent 8e08570 commit 07ebde9

File tree

3 files changed

+39
-15
lines changed

3 files changed

+39
-15
lines changed

offload/liboffload/include/OffloadImpl.hpp

+14
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "llvm/ADT/DenseSet.h"
2020
#include "llvm/ADT/StringRef.h"
2121
#include "llvm/ADT/StringSet.h"
22+
#include "llvm/Support/Error.h"
2223

2324
struct OffloadConfig {
2425
bool TracingEnabled = false;
@@ -88,6 +89,19 @@ struct ol_impl_result_t {
8889
Result = errors().emplace(std::move(Err)).first->get();
8990
}
9091

92+
static ol_impl_result_t fromError(llvm::Error &&Error) {
93+
ol_errc_t ErrCode;
94+
llvm::StringRef Details;
95+
llvm::handleAllErrors(std::move(Error), [&](llvm::StringError &Err) {
96+
// TODO: PluginInterface doesn't yet have a way to communicate offload
97+
// error codes
98+
ErrCode = OL_ERRC_UNKNOWN;
99+
Details = errorStrs().insert(Err.getMessage()).first->getKeyData();
100+
});
101+
102+
return ol_impl_result_t{ErrCode, Details};
103+
}
104+
91105
operator ol_result_t() { return Result; }
92106

93107
private:

offload/liboffload/src/OffloadImpl.cpp

+18-15
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,7 @@ ol_impl_result_t olMemAlloc_impl(ol_device_handle_t Device,
311311
auto Alloc =
312312
Device->Device->dataAlloc(Size, nullptr, convertOlToPluginAllocTy(Type));
313313
if (!Alloc)
314-
return {OL_ERRC_OUT_OF_RESOURCES,
315-
formatv("Could not create allocation on device {0}", Device).str()};
314+
return ol_impl_result_t::fromError(Alloc.takeError());
316315

317316
*AllocationOut = *Alloc;
318317
allocInfoMap().insert_or_assign(*Alloc, AllocInfo{Device, Type});
@@ -330,7 +329,7 @@ ol_impl_result_t olMemFree_impl(void *Address) {
330329
auto Res =
331330
Device->Device->dataDelete(Address, convertOlToPluginAllocTy(Type));
332331
if (Res)
333-
return {OL_ERRC_OUT_OF_RESOURCES, "Could not free allocation"};
332+
return ol_impl_result_t::fromError(std::move(Res));
334333

335334
allocInfoMap().erase(Address);
336335

@@ -342,7 +341,7 @@ ol_impl_result_t olCreateQueue_impl(ol_device_handle_t Device,
342341
auto CreatedQueue = std::make_unique<ol_queue_impl_t>(nullptr, Device);
343342
auto Err = Device->Device->initAsyncInfo(&(CreatedQueue->AsyncInfo));
344343
if (Err)
345-
return {OL_ERRC_UNKNOWN, "Could not initialize stream resource"};
344+
return ol_impl_result_t::fromError(std::move(Err));
346345

347346
*Queue = CreatedQueue.release();
348347
return OL_SUCCESS;
@@ -358,23 +357,23 @@ ol_impl_result_t olWaitQueue_impl(ol_queue_handle_t Queue) {
358357
if (Queue->AsyncInfo->Queue) {
359358
auto Err = Queue->Device->Device->synchronize(Queue->AsyncInfo);
360359
if (Err)
361-
return {OL_ERRC_INVALID_QUEUE, "The queue failed to synchronize"};
360+
return ol_impl_result_t::fromError(std::move(Err));
362361
}
363362

364363
// Recreate the stream resource so the queue can be reused
365364
// TODO: Would be easier for the synchronization to (optionally) not release
366365
// it to begin with.
367366
auto Res = Queue->Device->Device->initAsyncInfo(&Queue->AsyncInfo);
368367
if (Res)
369-
return {OL_ERRC_UNKNOWN, "Could not reinitialize the stream resource"};
368+
return ol_impl_result_t::fromError(std::move(Res));
370369

371370
return OL_SUCCESS;
372371
}
373372

374373
ol_impl_result_t olWaitEvent_impl(ol_event_handle_t Event) {
375374
auto Res = Event->Queue->Device->Device->syncEvent(Event->EventInfo);
376375
if (Res)
377-
return {OL_ERRC_INVALID_EVENT, "The event failed to synchronize"};
376+
return ol_impl_result_t::fromError(std::move(Res));
378377

379378
return OL_SUCCESS;
380379
}
@@ -390,13 +389,17 @@ ol_impl_result_t olDestroyEvent_impl(ol_event_handle_t Event) {
390389
ol_event_handle_t makeEvent(ol_queue_handle_t Queue) {
391390
auto EventImpl = std::make_unique<ol_event_impl_t>(nullptr, Queue);
392391
auto Res = Queue->Device->Device->createEvent(&EventImpl->EventInfo);
393-
if (Res)
392+
if (Res) {
393+
llvm::consumeError(std::move(Res));
394394
return nullptr;
395+
}
395396

396397
Res = Queue->Device->Device->recordEvent(EventImpl->EventInfo,
397398
Queue->AsyncInfo);
398-
if (Res)
399+
if (Res) {
400+
llvm::consumeError(std::move(Res));
399401
return nullptr;
402+
}
400403

401404
return EventImpl.release();
402405
}
@@ -422,16 +425,16 @@ ol_impl_result_t olMemcpy_impl(ol_queue_handle_t Queue, void *DstPtr,
422425
if (DstDevice == HostDevice()) {
423426
auto Res = SrcDevice->Device->dataRetrieve(DstPtr, SrcPtr, Size, QueueImpl);
424427
if (Res)
425-
return {OL_ERRC_UNKNOWN, "The data retrieve operation failed"};
428+
return ol_impl_result_t::fromError(std::move(Res));
426429
} else if (SrcDevice == HostDevice()) {
427430
auto Res = DstDevice->Device->dataSubmit(DstPtr, SrcPtr, Size, QueueImpl);
428431
if (Res)
429-
return {OL_ERRC_UNKNOWN, "The data submit operation failed"};
432+
return ol_impl_result_t::fromError(std::move(Res));
430433
} else {
431434
auto Res = SrcDevice->Device->dataExchange(SrcPtr, *DstDevice->Device,
432435
DstPtr, Size, QueueImpl);
433436
if (Res)
434-
return {OL_ERRC_UNKNOWN, "The data exchange operation failed"};
437+
return ol_impl_result_t::fromError(std::move(Res));
435438
}
436439

437440
if (EventOut)
@@ -459,7 +462,7 @@ ol_impl_result_t olCreateProgram_impl(ol_device_handle_t Device,
459462
Device->Device->loadBinary(Device->Device->Plugin, &Prog->DeviceImage);
460463
if (!Res) {
461464
delete Prog;
462-
return OL_ERRC_INVALID_VALUE;
465+
return ol_impl_result_t::fromError(Res.takeError());
463466
}
464467

465468
Prog->Image = *Res;
@@ -483,7 +486,7 @@ ol_impl_result_t olGetKernel_impl(ol_program_handle_t Program,
483486

484487
auto Err = KernelImpl->init(Device, *Program->Image);
485488
if (Err)
486-
return {OL_ERRC_UNKNOWN, "Could not initialize the kernel"};
489+
return ol_impl_result_t::fromError(std::move(Err));
487490

488491
*Kernel = &*KernelImpl;
489492

@@ -526,7 +529,7 @@ olLaunchKernel_impl(ol_queue_handle_t Queue, ol_device_handle_t Device,
526529

527530
AsyncInfoWrapper.finalize(Err);
528531
if (Err)
529-
return {OL_ERRC_UNKNOWN, "Could not finalize the AsyncInfoWrapper"};
532+
return ol_impl_result_t::fromError(std::move(Err));
530533

531534
if (EventOut)
532535
*EventOut = makeEvent(Queue);

offload/unittests/OffloadAPI/kernel/olGetKernel.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,10 @@ TEST_P(olGetKernelTest, InvalidNullKernelPointer) {
2929
ASSERT_ERROR(OL_ERRC_INVALID_NULL_POINTER,
3030
olGetKernel(Program, "foo", nullptr));
3131
}
32+
33+
// Error code returning from plugin interface not yet supported
34+
TEST_F(olGetKernelTest, DISABLED_InvalidKernelName) {
35+
ol_kernel_handle_t Kernel = nullptr;
36+
ASSERT_ERROR(OL_ERRC_INVALID_KERNEL_NAME,
37+
olGetKernel(Program, "invalid_kernel_name", &Kernel));
38+
}

0 commit comments

Comments
 (0)