Skip to content

Commit

Permalink
[OpenMP] Make isDone lightweight without calling synchronize
Browse files Browse the repository at this point in the history
~TaskAsyncInfoWrapperTy() calls isDone. With synchronize inside isDone, we need to handle the error return from synchronize in the destructor.
The consumers of TaskAsyncInfoWrapperTy, targetDataMapper and targetKernel, both call AsyncInfo.synchronize() before exiting.
For this reason in ~TaskAsyncInfoWrapperTy(), calling synchronize() via isDone() is redundant.
This patch removes synchronize() call inside isDone() and makes it a lightweight check.
__tgt_target_nowait_query needs to call synchronize() before checking isDone().

Differential Revision: https://reviews.llvm.org/D144315

(cherry picked from commit e2069be)
  • Loading branch information
ye-luo authored and tru committed Feb 20, 2023
1 parent f57f59e commit babeb69
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 20 deletions.
9 changes: 4 additions & 5 deletions openmp/libomptarget/include/omptarget.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include <cstdint>
#include <deque>
#include <functional>
#include <optional>
#include <stddef.h>
#include <stdint.h>
#include <type_traits>
Expand Down Expand Up @@ -244,12 +243,12 @@ class AsyncInfoTy {

/// Check if all asynchronous operations are completed.
///
/// \note if the operations are completed, the registered post-processing
/// functions will be executed once and unregistered afterwards.
/// \note only a lightweight check. If needed, use synchronize() to query the
/// status of AsyncInfo before checking.
///
/// \returns true if there is no pending asynchronous operations, false
/// otherwise. We return a null value in the case of an error from the plugin.
std::optional<bool> isDone();
/// otherwise.
bool isDone() const;

/// Add a new post-processing function to be executed after synchronization.
///
Expand Down
5 changes: 2 additions & 3 deletions openmp/libomptarget/src/interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -412,12 +412,11 @@ EXTERN void __tgt_target_nowait_query(void **AsyncHandle) {
if (QueryCounter.isAboveThreshold())
AsyncInfo->SyncType = AsyncInfoTy::SyncTy::BLOCKING;

auto DoneOrErr = AsyncInfo->isDone();
if (!DoneOrErr)
if (const int Rc = AsyncInfo->synchronize())
FATAL_MESSAGE0(1, "Error while querying the async queue for completion.\n");
// If there are device operations still pending, return immediately without
// deallocating the handle and increase the current thread query count.
if (!*DoneOrErr) {
if (!AsyncInfo->isDone()) {
QueryCounter.increment();
return;
}
Expand Down
8 changes: 1 addition & 7 deletions openmp/libomptarget/src/omptarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,7 @@ void *&AsyncInfoTy::getVoidPtrLocation() {
return BufferLocations.back();
}

std::optional<bool> AsyncInfoTy::isDone() {
if (synchronize() == OFFLOAD_FAIL)
return std::nullopt;

// The async info operations are completed when the internal queue is empty.
return isQueueEmpty();
}
bool AsyncInfoTy::isDone() const { return isQueueEmpty(); }

int32_t AsyncInfoTy::runPostProcessing() {
size_t Size = PostProcessingFunctions.size();
Expand Down
6 changes: 1 addition & 5 deletions openmp/libomptarget/src/private.h
Original file line number Diff line number Diff line change
Expand Up @@ -250,13 +250,9 @@ class TaskAsyncInfoWrapperTy {
if (AsyncInfo == &LocalAsyncInfo)
return;

auto DoneOrErr = AsyncInfo->isDone();
if (!DoneOrErr)
FATAL_MESSAGE0(1,
"Error while querying the async queue for completion.\n");
// If the are device operations still pending, return immediately without
// deallocating the handle.
if (!*DoneOrErr)
if (!AsyncInfo->isDone())
return;

// Delete the handle and unset it from the OpenMP task data.
Expand Down

0 comments on commit babeb69

Please sign in to comment.