[SYCL] Don't throw exceptions from destructors (#1378)

SYCL RT can throw exceptions in case of enqueue process failure.
Some commands are scheduled while destructor call. So possible
exceptions should be handlen by destructor since throwing
exceptions out of a destructor is undefined behaviour.

Signed-off-by: Ivan Karachun <[email protected]>

Author: Ivan Karachun

sycl/include/CL/sycl/detail/buffer_impl.hpp

@@ -108,7 +108,12 @@ class buffer_impl final : public SYCLMemObjT {
 
   MemObjType getType() const override { return MemObjType::BUFFER; }
 
-  ~buffer_impl() { BaseT::updateHostMemory(); }
+  ~buffer_impl() {
+    try {
+      BaseT::updateHostMemory();
+    } catch (...) {
+    }
+  }
 };
 
 } // namespace detail

sycl/include/CL/sycl/detail/image_impl.hpp

@@ -216,7 +216,12 @@ template <int Dimensions> class image_impl final : public SYCLMemObjT {
 
   size_t getSlicePitch() const { return MSlicePitch; }
 
-  ~image_impl() { BaseT::updateHostMemory(); }
+  ~image_impl() {
+    try {
+      BaseT::updateHostMemory();
+    } catch (...) {
+    }
+  }
 
 private:
   vector_class<device> getDevices(const ContextImplPtr Context);

sycl/source/detail/accessor_impl.cpp

@@ -15,8 +15,11 @@ namespace sycl {
 namespace detail {
 
 AccessorImplHost::~AccessorImplHost() {
-  if (MBlockedCmd)
-    detail::Scheduler::getInstance().releaseHostAccessor(this);
+  try {
+    if (MBlockedCmd)
+      detail::Scheduler::getInstance().releaseHostAccessor(this);
+  } catch (...) {
+  }
 }
 
 void addHostAccessorAndWait(Requirement *Req) {

sycl/source/detail/scheduler/commands.cpp

@@ -228,10 +228,11 @@ void Command::waitForEvents(QueueImplPtr Queue,
 }
 
 Command::Command(CommandType Type, QueueImplPtr Queue)
-    : MQueue(std::move(Queue)), MType(Type), MEnqueued(false) {
+    : MQueue(std::move(Queue)), MType(Type) {
   MEvent.reset(new detail::event_impl(MQueue));
   MEvent->setCommand(this);
   MEvent->setContextImpl(detail::getSyclObjImpl(MQueue->get_context()));
+  MEnqueueStatus = EnqueueResultT::SyclEnqueueReady;
 
 #ifdef XPTI_ENABLE_INSTRUMENTATION
   if (!xptiTraceEnabled())
@@ -451,11 +452,11 @@ void Command::emitInstrumentation(uint16_t Type, const char *Txt) {
 
 bool Command::enqueue(EnqueueResultT &EnqueueResult, BlockingT Blocking) {
   // Exit if already enqueued
-  if (MEnqueued)
+  if (MEnqueueStatus == EnqueueResultT::SyclEnqueueSuccess)
     return true;
 
   // If the command is blocked from enqueueing
-  if (MIsBlockable && !MCanEnqueue) {
+  if (MIsBlockable && MEnqueueStatus == EnqueueResultT::SyclEnqueueBlocked) {
     // Exit if enqueue type is not blocking
     if (!Blocking) {
       EnqueueResult = EnqueueResultT(EnqueueResultT::SyclEnqueueBlocked, this);
@@ -478,7 +479,7 @@ bool Command::enqueue(EnqueueResultT &EnqueueResult, BlockingT Blocking) {
 #endif
 
     // Wait if blocking
-    while (!MCanEnqueue)
+    while (MEnqueueStatus == EnqueueResultT::SyclEnqueueBlocked)
       ;
 #ifdef XPTI_ENABLE_INSTRUMENTATION
     emitInstrumentation(xpti::trace_barrier_end, Info.c_str());
@@ -488,13 +489,22 @@ bool Command::enqueue(EnqueueResultT &EnqueueResult, BlockingT Blocking) {
   std::lock_guard<std::mutex> Lock(MEnqueueMtx);
 
   // Exit if the command is already enqueued
-  if (MEnqueued)
+  if (MEnqueueStatus == EnqueueResultT::SyclEnqueueSuccess)
     return true;
 
 #ifdef XPTI_ENABLE_INSTRUMENTATION
   emitInstrumentation(xpti::trace_task_begin, nullptr);
 #endif
 
+  if (MEnqueueStatus == EnqueueResultT::SyclEnqueueFailed) {
+    EnqueueResult = EnqueueResultT(EnqueueResultT::SyclEnqueueFailed, this);
+    return false;
+  }
+
+  // Command status set to "failed" beforehand, so this command
+  // has already been marked as "failed" if enqueueImp throws an exception.
+  // This will avoid execution of the same failed command twice.
+  MEnqueueStatus = EnqueueResultT::SyclEnqueueFailed;
   cl_int Res = enqueueImp();
 
   if (CL_SUCCESS != Res)
@@ -503,14 +513,14 @@ bool Command::enqueue(EnqueueResultT &EnqueueResult, BlockingT Blocking) {
   else
     // Consider the command is successfully enqueued if return code is
     // CL_SUCCESS
-    MEnqueued = true;
+    MEnqueueStatus = EnqueueResultT::SyclEnqueueSuccess;
 
   // Emit this correlation signal before the task end
   emitEnqueuedEventSignal(MEvent->getHandleRef());
 #ifdef XPTI_ENABLE_INSTRUMENTATION
   emitInstrumentation(xpti::trace_task_end, nullptr);
 #endif
-  return static_cast<bool>(MEnqueued);
+  return MEnqueueStatus == EnqueueResultT::SyclEnqueueSuccess;
 }
 
 void Command::resolveReleaseDependencies(std::set<Command *> &DepList) {

sycl/source/detail/scheduler/commands.hpp

@@ -40,7 +40,12 @@ enum BlockingT { NON_BLOCKING = 0, BLOCKING };
 
 // The struct represents the result of command enqueueing
 struct EnqueueResultT {
-  enum ResultT { SyclEnqueueSuccess, SyclEnqueueBlocked, SyclEnqueueFailed };
+  enum ResultT {
+    SyclEnqueueReady,
+    SyclEnqueueSuccess,
+    SyclEnqueueBlocked,
+    SyclEnqueueFailed
+  };
   EnqueueResultT(ResultT Result = SyclEnqueueSuccess, Command *Cmd = nullptr,
                  cl_int ErrCode = CL_SUCCESS)
       : MResult(Result), MCmd(Cmd), MErrCode(ErrCode) {}
@@ -110,7 +115,9 @@ class Command {
 
   bool isFinished();
 
-  bool isEnqueued() const { return MEnqueued; }
+  bool isSuccessfullyEnqueued() const {
+    return MEnqueueStatus == EnqueueResultT::SyclEnqueueSuccess;
+  }
 
   std::shared_ptr<queue_impl> getQueue() const { return MQueue; }
 
@@ -170,8 +177,6 @@ class Command {
 
   // The type of the command
   CommandType MType;
-  // Indicates whether the command is enqueued or not
-  std::atomic<bool> MEnqueued;
   // Mutex used to protect enqueueing from race conditions
   std::mutex MEnqueueMtx;
 
@@ -182,13 +187,14 @@ class Command {
   std::unordered_set<Command *> MUsers;
   // Indicates whether the command can be blocked from enqueueing
   bool MIsBlockable = false;
-  // Indicates whether the command is blocked from enqueueing
-  std::atomic<bool> MCanEnqueue;
   // Counts the number of memory objects this command is a leaf for
   unsigned MLeafCounter = 0;
 
   const char *MBlockReason = "Unknown";
 
+  // Describes the status of a command
+  std::atomic<EnqueueResultT::ResultT> MEnqueueStatus;
+
   // All member variable defined here  are needed for the SYCL instrumentation
   // layer. Do not guard these variables below with XPTI_ENABLE_INSTRUMENTATION
   // to ensure we have the same object layout when the macro in the library and

sycl/source/detail/scheduler/graph_builder.cpp

@@ -426,7 +426,7 @@ Command *Scheduler::GraphBuilder::addHostAccessor(Requirement *Req,
   UpdateHostAccCmd->addUser(EmptyCmd);
 
   EmptyCmd->MIsBlockable = true;
-  EmptyCmd->MCanEnqueue = false;
+  EmptyCmd->MEnqueueStatus = EnqueueResultT::SyclEnqueueBlocked;
   EmptyCmd->MBlockReason = "A Buffer is locked by the host accessor";
 
   updateLeaves({UpdateHostAccCmd}, Record, Req->MAccessMode);

sycl/source/detail/scheduler/graph_processor.cpp

@@ -55,7 +55,7 @@ void Scheduler::GraphProcessor::waitForEvent(EventImplPtr Event) {
 bool Scheduler::GraphProcessor::enqueueCommand(Command *Cmd,
                                                EnqueueResultT &EnqueueResult,
                                                BlockingT Blocking) {
-  if (!Cmd || Cmd->isEnqueued())
+  if (!Cmd || Cmd->isSuccessfullyEnqueued())
     return true;
 
   // Indicates whether dependency cannot be enqueued

sycl/source/detail/scheduler/scheduler.cpp

@@ -177,7 +177,7 @@ EventImplPtr Scheduler::addHostAccessor(Requirement *Req,
 }
 
 void Scheduler::releaseHostAccessor(Requirement *Req) {
-  Req->MBlockedCmd->MCanEnqueue = true;
+  Req->MBlockedCmd->MEnqueueStatus = EnqueueResultT::SyclEnqueueReady;
   MemObjRecord* Record = Req->MSYCLMemObj->MRecord.get();
   auto EnqueueLeaves = [](CircularBuffer<Command *> &Leaves) {
     for (Command *Cmd : Leaves) {

sycl/test/scheduler/HandleException.cpp

@@ -0,0 +1,50 @@
+// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple -I %sycl_source_dir %s -o %t.out
+// RUN: %CPU_RUN_PLACEHOLDER %t.out
+#include <CL/sycl.hpp>
+#include <array>
+
+using namespace cl::sycl;
+
+constexpr access::mode sycl_read = access::mode::read;
+constexpr access::mode sycl_write = access::mode::write;
+
+constexpr unsigned MAX_WG_SIZE = 4;
+constexpr unsigned SIZE = 5;
+using ArrayType = std::array<unsigned, SIZE>;
+
+class kernelCompute;
+
+// Return 'true' if an exception was thrown.
+bool run_kernel(const unsigned wg_size) {
+  ArrayType index;
+  const unsigned N = index.size();
+  {
+    buffer<cl_uint, 1> bufferIdx(index.data(), N);
+    queue deviceQueue;
+    try {
+      deviceQueue.submit([&](handler &cgh) {
+        auto accessorIdx = bufferIdx.get_access<sycl_read>(cgh);
+        cgh.parallel_for<class kernelCompute>(
+            nd_range<1>(range<1>(N), range<1>(wg_size)),
+            [=](nd_item<1> ID) [[cl::reqd_work_group_size(1, 1, MAX_WG_SIZE)]] {
+              (void)accessorIdx[ID.get_global_id(0)];
+            });
+      });
+    } catch (nd_range_error &err) {
+      return true;
+    } catch (...) {
+      assert(!"Unknown exception was thrown");
+    }
+  }
+  return false;
+}
+
+int main() {
+  bool success_exception = run_kernel(MAX_WG_SIZE);
+  assert(!success_exception &&
+         "Unexpected exception was thrown for success call");
+  bool fail_exception = run_kernel(SIZE);
+  assert(fail_exception && "No exception was thrown");
+
+  return 0;
+}

sycl/unittests/scheduler/BlockedCommands.cpp

@@ -41,8 +41,8 @@ class TestScheduler : public detail::Scheduler {
 TEST_F(SchedulerTest, BlockedCommands) {
   MockCommand MockCmd(detail::getSyclObjImpl(MQueue));
 
+  MockCmd.MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueBlocked;
   MockCmd.MIsBlockable = true;
-  MockCmd.MCanEnqueue = false;
   MockCmd.MRetVal = CL_DEVICE_PARTITION_EQUALLY;
 
   detail::EnqueueResultT Res;
@@ -52,7 +52,7 @@ TEST_F(SchedulerTest, BlockedCommands) {
   ASSERT_EQ(detail::EnqueueResultT::SyclEnqueueBlocked, Res.MResult)
       << "Result of enqueueing blocked command should be BLOCKED\n";
 
-  MockCmd.MCanEnqueue = true;
+  MockCmd.MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueReady;
   Res.MResult = detail::EnqueueResultT::SyclEnqueueSuccess;
   MockCmd.MRetVal = CL_DEVICE_PARTITION_EQUALLY;
 
@@ -65,6 +65,7 @@ TEST_F(SchedulerTest, BlockedCommands) {
   ASSERT_EQ(&MockCmd, Res.MCmd) << "Expected different failed command.\n";
 
   Res = detail::EnqueueResultT{};
+  MockCmd.MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueReady;
   MockCmd.MRetVal = CL_SUCCESS;
   Enqueued = TestScheduler::enqueueCommand(&MockCmd, Res, detail::BLOCKING);
   ASSERT_TRUE(Enqueued &&

sycl/unittests/scheduler/CMakeLists.txt

@@ -8,6 +8,7 @@ set(CMAKE_CXX_COMPILER ${clang})
 
 add_sycl_unittest(SchedulerTests
     BlockedCommands.cpp
+    FailedCommands.cpp
     FinishedCmdCleanup.cpp
     LeafLimit.cpp
     MemObjCommandCleanup.cpp

sycl/unittests/scheduler/FailedCommands.cpp

@@ -0,0 +1,61 @@
+//==----------- FailedCommands.cpp ---- Scheduler unit tests ---------------==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SchedulerTest.hpp"
+
+#include <CL/cl.h>
+#include <CL/sycl.hpp>
+#include <detail/scheduler/scheduler.hpp>
+
+#include <gtest/gtest.h>
+
+using namespace cl::sycl;
+
+class MockCommand : public detail::Command {
+public:
+  MockCommand(detail::QueueImplPtr Queue)
+      : Command(detail::Command::ALLOCA, Queue) {}
+  void printDot(std::ostream &Stream) const override {}
+  void emitInstrumentationData() override {}
+  cl_int enqueueImp() override { return CL_SUCCESS; }
+};
+
+class MockScheduler : public detail::Scheduler {
+public:
+  static bool enqueueCommand(detail::Command *Cmd,
+                             detail::EnqueueResultT &EnqueueResult,
+                             detail::BlockingT Blocking) {
+    return GraphProcessor::enqueueCommand(Cmd, EnqueueResult, Blocking);
+  }
+};
+
+TEST_F(SchedulerTest, FailedDependency) {
+  detail::Requirement MockReq(/*Offset*/ {0, 0, 0}, /*AccessRange*/ {1, 1, 1},
+                              /*MemoryRange*/ {1, 1, 1},
+                              access::mode::read_write, /*SYCLMemObjT*/ nullptr,
+                              /*Dims*/ 1, /*ElementSize*/ 1);
+  MockCommand MDep(detail::getSyclObjImpl(MQueue));
+  MockCommand MUser(detail::getSyclObjImpl(MQueue));
+  MDep.addUser(&MUser);
+  MUser.addDep(detail::DepDesc{&MDep, &MockReq, nullptr});
+  MUser.MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueReady;
+  MDep.MEnqueueStatus = detail::EnqueueResultT::SyclEnqueueFailed;
+
+  detail::EnqueueResultT Res;
+  bool Enqueued =
+      MockScheduler::enqueueCommand(&MUser, Res, detail::NON_BLOCKING);
+
+  ASSERT_FALSE(Enqueued) << "Enqueue process must fail\n";
+  ASSERT_EQ(Res.MCmd, &MDep) << "Wrong failed command\n";
+  ASSERT_EQ(Res.MResult, detail::EnqueueResultT::SyclEnqueueFailed)
+      << "Enqueue process must fail\n";
+  ASSERT_EQ(MUser.MEnqueueStatus, detail::EnqueueResultT::SyclEnqueueReady)
+      << "MUser shouldn't be marked as failed\n";
+  ASSERT_EQ(MDep.MEnqueueStatus, detail::EnqueueResultT::SyclEnqueueFailed)
+      << "MDep should be marked as failed\n";
+}