[Openmp-commits] [PATCH] D144191: [Libomptarget] Check errors when synchronizing the async queue

[Ye Luo via Phabricator via Openmp-commits]

ye-luo added inline comments.


================
Comment at: openmp/libomptarget/src/interface.cpp:415
 
+  auto DoneOrErr = AsyncInfo->isDone();
+  if (!DoneOrErr)
----------------
ye-luo wrote:
> Prefer DoneStatusOrError. Unclear how DoneOrErr encapsulate NotDones state.
Changed my mind. Shorter name is fine.


================
Comment at: openmp/libomptarget/src/omptarget.cpp:55
+std::optional<bool> AsyncInfoTy::isDone() {
+  if (int Result = synchronize())
+    return std::nullopt;
----------------
ye-luo wrote:
> synchronize returns OFFLOAD_SUCCESS which has value 0.
> quite hard to get through this logic by just reading the above line.
> Why not 
> ```
> if (synchronize() == OFFLOAD_SUCCESS)
>   return isQueueEmpty();
> return std::nullopt;
> ```
> 
Got unhappy GNU.
```
/home/yeluo/opt/llvm-clang/llvm-project/openmp/libomptarget/src/omptarget.cpp: In member function 'std::optional<bool> AsyncInfoTy::isDone()':
/home/yeluo/opt/llvm-clang/llvm-project/openmp/libomptarget/src/omptarget.cpp:55:11: warning: unused variable 'Result' [-Wunused-variable]
   55 |   if (int Result = synchronize())
```


================
Comment at: openmp/libomptarget/src/private.h:255
+    if (!DoneOrErr)
+      FATAL_MESSAGE0(1, "Error while synchronizing the async queue\n");
     // If the are device operations still pending, return immediately without
----------------
This message confused me "Error while query the async queue completion.\n"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144191/new/

https://reviews.llvm.org/D144191