Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion offload/liboffload/API/Event.td
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ def : Enum {
let desc = "Supported event info.";
let is_typed = 1;
let etors = [
TaggedEtor<"QUEUE", "ol_queue_handle_t", "The handle of the queue associated with the device.">
TaggedEtor<"QUEUE", "ol_queue_handle_t", "The handle of the queue associated with the device.">,
TaggedEtor<"IS_COMPLETE", "bool", "True if and only if the event is complete.">,
];
}

Expand Down
16 changes: 14 additions & 2 deletions offload/liboffload/src/OffloadImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -553,8 +553,8 @@ Error olGetQueueInfoSize_impl(ol_queue_handle_t Queue, ol_queue_info_t PropName,
}

Error olSyncEvent_impl(ol_event_handle_t Event) {
// No event info means that this event was complete on creation
if (!Event->EventInfo)
// Event always complete
return Plugin::success();

if (auto Res = Event->Queue->Device->Device->syncEvent(Event->EventInfo))
Expand All @@ -575,10 +575,22 @@ Error olGetEventInfoImplDetail(ol_event_handle_t Event,
ol_event_info_t PropName, size_t PropSize,
void *PropValue, size_t *PropSizeRet) {
InfoWriter Info(PropSize, PropValue, PropSizeRet);
auto Queue = Event->Queue;

switch (PropName) {
case OL_EVENT_INFO_QUEUE:
return Info.write<ol_queue_handle_t>(Event->Queue);
return Info.write<ol_queue_handle_t>(Queue);
case OL_EVENT_INFO_IS_COMPLETE: {
// No event info means that this event was complete on creation
if (!Event->EventInfo)
return Info.write<bool>(true);

auto Res = Queue->Device->Device->isEventComplete(Event->EventInfo,
Queue->AsyncInfo);
if (auto Err = Res.takeError())
return Err;
return Info.write<bool>(*Res);
}
default:
return createOffloadError(ErrorCode::INVALID_ENUMERATION,
"olGetEventInfo enum '%i' is invalid", PropName);
Expand Down
22 changes: 22 additions & 0 deletions offload/plugins-nextgen/amdgpu/src/rtl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1519,6 +1519,9 @@ struct AMDGPUStreamTy {
/// actions for that and prior events.
Error synchronizeOn(AMDGPUEventTy &Event);

/// Return true if the event from this queue is complete
Expected<bool> isEventComplete(const AMDGPUEventTy &Event);

/// Query the stream and complete pending post actions if operations finished.
/// Return whether all the operations completed. This operation does not block
/// the calling thread.
Expand Down Expand Up @@ -1683,6 +1686,18 @@ Error AMDGPUStreamTy::synchronizeOn(AMDGPUEventTy &Event) {
return completeUntil(Event.RecordedSlot);
}

Expected<bool> AMDGPUStreamTy::isEventComplete(const AMDGPUEventTy &Event) {
std::lock_guard<std::mutex> Lock(Mutex);
assert(Event.RecordedStream == this && "event is for a different stream");

if (Event.RecordedSyncCycle < SyncCycle) {
return true;
}
assert(Event.RecordedSyncCycle == SyncCycle && "event is from the future?");

return !Slots[Event.RecordedSlot].Signal->load();
}

struct AMDGPUStreamManagerTy final
: GenericDeviceResourceManagerTy<AMDGPUResourceRef<AMDGPUStreamTy>> {
using ResourceRef = AMDGPUResourceRef<AMDGPUStreamTy>;
Expand Down Expand Up @@ -2601,6 +2616,13 @@ struct AMDGPUDeviceTy : public GenericDeviceTy, AMDGenericDeviceTy {
return Query.takeError();
}

Expected<bool> isEventCompleteImpl(void *EventPtr,
AsyncInfoWrapperTy &AsyncInfo) override {
AMDGPUEventTy *Event = reinterpret_cast<AMDGPUEventTy *>(EventPtr);
auto Stream = AsyncInfo.getQueueAs<AMDGPUStreamTy *>();
return Stream && Stream->isEventComplete(*Event);
}

/// Synchronize the current thread with the event.
Error syncEventImpl(void *EventPtr) override {
AMDGPUEventTy *Event = reinterpret_cast<AMDGPUEventTy *>(EventPtr);
Expand Down
5 changes: 5 additions & 0 deletions offload/plugins-nextgen/common/include/PluginInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,11 @@ struct GenericDeviceTy : public DeviceAllocatorTy {
virtual Error waitEventImpl(void *EventPtr,
AsyncInfoWrapperTy &AsyncInfoWrapper) = 0;

/// Check if the event enqueued to AsyncInfo is complete
Expected<bool> isEventComplete(void *Event, __tgt_async_info *AsyncInfo);
virtual Expected<bool>
isEventCompleteImpl(void *EventPtr, AsyncInfoWrapperTy &AsyncInfoWrapper) = 0;

/// Synchronize the current thread with the event.
Error syncEvent(void *EventPtr);
virtual Error syncEventImpl(void *EventPtr) = 0;
Expand Down
16 changes: 16 additions & 0 deletions offload/plugins-nextgen/common/src/PluginInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1648,6 +1648,22 @@ Expected<bool> GenericDeviceTy::hasPendingWork(__tgt_async_info *AsyncInfo) {
return Res;
}

Expected<bool> GenericDeviceTy::isEventComplete(void *Event,
__tgt_async_info *AsyncInfo) {
AsyncInfoWrapperTy AsyncInfoWrapper(*this, AsyncInfo);
auto Res = isEventCompleteImpl(Event, AsyncInfoWrapper);
if (auto Err = Res.takeError()) {
AsyncInfoWrapper.finalize(Err);
return Err;
}

auto Err = Plugin::success();
AsyncInfoWrapper.finalize(Err);
if (Err)
return Err;
return Res;
}

Error GenericDeviceTy::syncEvent(void *EventPtr) {
return syncEventImpl(EventPtr);
}
Expand Down
1 change: 1 addition & 0 deletions offload/plugins-nextgen/cuda/dynamic_cuda/cuda.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ DLWRAP(cuCtxSetLimit, 2)

DLWRAP(cuEventCreate, 2)
DLWRAP(cuEventRecord, 2)
DLWRAP(cuEventQuery, 1)
DLWRAP(cuStreamWaitEvent, 3)
DLWRAP(cuEventSynchronize, 1)
DLWRAP(cuEventDestroy, 1)
Expand Down
1 change: 1 addition & 0 deletions offload/plugins-nextgen/cuda/dynamic_cuda/cuda.h
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ CUresult cuCtxSetLimit(CUlimit, size_t);

CUresult cuEventCreate(CUevent *, unsigned int);
CUresult cuEventRecord(CUevent, CUstream);
CUresult cuEventQuery(CUevent);
CUresult cuStreamWaitEvent(CUstream, CUevent, unsigned int);
CUresult cuEventSynchronize(CUevent);
CUresult cuEventDestroy(CUevent);
Expand Down
14 changes: 14 additions & 0 deletions offload/plugins-nextgen/cuda/src/rtl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,20 @@ struct CUDADeviceTy : public GenericDeviceTy {
return true;
}

Expected<bool> isEventCompleteImpl(void *EventPtr,
AsyncInfoWrapperTy &) override {
CUevent Event = reinterpret_cast<CUevent>(EventPtr);

CUresult Ret = cuEventQuery(Event);
if (Ret == CUDA_SUCCESS)
return true;

if (Ret == CUDA_ERROR_NOT_READY)
return false;

return Plugin::check(Ret, "error in cuEventQuery: %s");
}

/// Synchronize the current thread with the event.
Error syncEventImpl(void *EventPtr) override {
CUevent Event = reinterpret_cast<CUevent>(EventPtr);
Expand Down
4 changes: 4 additions & 0 deletions offload/plugins-nextgen/host/src/rtl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,10 @@ struct GenELF64DeviceTy : public GenericDeviceTy {
Expected<bool> hasPendingWorkImpl(AsyncInfoWrapperTy &AsyncInfo) override {
return true;
}
Expected<bool> isEventCompleteImpl(void *Event,
AsyncInfoWrapperTy &AsyncInfo) override {
return true;
}
Error syncEventImpl(void *EventPtr) override { return Plugin::success(); }

/// Print information about the device.
Expand Down
11 changes: 10 additions & 1 deletion offload/unittests/OffloadAPI/event/olGetEventInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,22 @@
using olGetEventInfoTest = OffloadEventTest;
OFFLOAD_TESTS_INSTANTIATE_DEVICE_FIXTURE(olGetEventInfoTest);

TEST_P(olGetEventInfoTest, SuccessDevice) {
TEST_P(olGetEventInfoTest, SuccessQueue) {
ol_queue_handle_t RetrievedQueue;
ASSERT_SUCCESS(olGetEventInfo(Event, OL_EVENT_INFO_QUEUE,
sizeof(ol_queue_handle_t), &RetrievedQueue));
ASSERT_EQ(Queue, RetrievedQueue);
}

TEST_P(olGetEventInfoTest, SuccessIsComplete) {
bool Complete = false;
while (!Complete) {
ASSERT_SUCCESS(olGetEventInfo(Event, OL_EVENT_INFO_IS_COMPLETE,
sizeof(Complete), &Complete));
}
ASSERT_EQ(Complete, true);
}

TEST_P(olGetEventInfoTest, InvalidNullHandle) {
ol_queue_handle_t RetrievedQueue;
ASSERT_ERROR(OL_ERRC_INVALID_NULL_HANDLE,
Expand Down
6 changes: 6 additions & 0 deletions offload/unittests/OffloadAPI/event/olGetEventInfoSize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ TEST_P(olGetEventInfoSizeTest, SuccessQueue) {
ASSERT_EQ(Size, sizeof(ol_queue_handle_t));
}

TEST_P(olGetEventInfoSizeTest, SuccessIsComplete) {
size_t Size = 0;
ASSERT_SUCCESS(olGetEventInfoSize(Event, OL_EVENT_INFO_IS_COMPLETE, &Size));
ASSERT_EQ(Size, sizeof(bool));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ASSERT_EQ(Size, sizeof(bool));
EXPECT_EQ(Size, sizeof(bool));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason why? We don't use ASSERT anywhere else in the unit tests and it doesn't make sense for the function to continue running if this fails.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EXPECT_* should be preferred unless failure will result in later checks crashing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which case, if we want to change over to that, I think it makes sense to do that as a single separate change that touches all the tests. I think we should use ASSERT here now for the sake of consistency with other tests.

}

TEST_P(olGetEventInfoSizeTest, InvalidNullHandle) {
size_t Size = 0;
ASSERT_ERROR(OL_ERRC_INVALID_NULL_HANDLE,
Expand Down
Loading