From bba656e32f3ab6f6abedbc110320c4ec7271d949 Mon Sep 17 00:00:00 2001 From: "Larsen, Steffen" Date: Mon, 5 May 2025 04:47:01 -0700 Subject: [PATCH 1/3] [SYCL] Simplify secondary queue usage The requirement of SYCL submit functions taking a secondary queue does not mandate more than that the runtime must attempt to enqueue onto the secondary queue if the it fails to enqueue on the primary queue. Using this, the SYCL implementation can be simplified to simply retry the enqueue if the enqueue to the primary queue throws an exception. Signed-off-by: Larsen, Steffen --- .../oneapi/experimental/enqueue_functions.hpp | 2 +- sycl/include/sycl/handler.hpp | 4 +- sycl/include/sycl/queue.hpp | 67 ++++----------- sycl/source/detail/handler_impl.hpp | 6 ++ sycl/source/detail/queue_impl.cpp | 14 +++- sycl/source/detail/queue_impl.hpp | 25 +++++- sycl/source/detail/scheduler/scheduler.hpp | 8 -- sycl/source/handler.cpp | 11 ++- sycl/source/queue.cpp | 2 + sycl/unittests/SYCL2020/KernelBundle.cpp | 82 ++++++++----------- 10 files changed, 103 insertions(+), 118 deletions(-) diff --git a/sycl/include/sycl/ext/oneapi/experimental/enqueue_functions.hpp b/sycl/include/sycl/ext/oneapi/experimental/enqueue_functions.hpp index f4a102b235538..21cc91d621b40 100644 --- a/sycl/include/sycl/ext/oneapi/experimental/enqueue_functions.hpp +++ b/sycl/include/sycl/ext/oneapi/experimental/enqueue_functions.hpp @@ -108,7 +108,7 @@ event submit_with_event_impl(queue &Q, PropertiesT Props, CommandGroupFunc &&CGF, const sycl::detail::code_location &CodeLoc) { return Q.submit_with_event<__SYCL_USE_FALLBACK_ASSERT>( - Props, detail::type_erased_cgfo_ty{CGF}, nullptr, CodeLoc); + Props, detail::type_erased_cgfo_ty{CGF}, CodeLoc); } } // namespace detail diff --git a/sycl/include/sycl/handler.hpp b/sycl/include/sycl/handler.hpp index e2657396bfa06..d43dfa45697d3 100644 --- a/sycl/include/sycl/handler.hpp +++ b/sycl/include/sycl/handler.hpp @@ -428,6 +428,7 @@ class __SYCL_EXPORT handler { /// is needed by the caller. handler(std::shared_ptr Queue, bool CallerNeedsEvent); +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES /// Constructs SYCL handler from the associated queue and the submission's /// primary and secondary queue. /// @@ -438,17 +439,16 @@ class __SYCL_EXPORT handler { /// is null if no secondary queue is associated with the submission. /// \param CallerNeedsEvent indicates if the event resulting from this handler /// is needed by the caller. -#ifndef __INTEL_PREVIEW_BREAKING_CHANGES // TODO: This function is not used anymore, remove it in the next // ABI-breaking window. handler(std::shared_ptr Queue, std::shared_ptr PrimaryQueue, std::shared_ptr SecondaryQueue, bool CallerNeedsEvent); -#endif __SYCL_DLL_LOCAL handler(std::shared_ptr Queue, detail::queue_impl *SecondaryQueue, bool CallerNeedsEvent); +#endif /// Constructs SYCL handler from Graph. /// diff --git a/sycl/include/sycl/queue.hpp b/sycl/include/sycl/queue.hpp index c9880ae7efcbe..b1d6bab51e8fe 100644 --- a/sycl/include/sycl/queue.hpp +++ b/sycl/include/sycl/queue.hpp @@ -66,7 +66,7 @@ auto get_native(const SyclObjectT &Obj) namespace detail { class queue_impl; -inline event submitAssertCapture(queue &, event &, queue *, +inline event submitAssertCapture(queue &, event &, const detail::code_location &); // Function to postprocess submitted command @@ -86,8 +86,10 @@ class __SYCL_EXPORT SubmissionInfo { sycl::detail::optional &PostProcessorFunc(); const sycl::detail::optional &PostProcessorFunc() const; +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES std::shared_ptr &SecondaryQueue(); const std::shared_ptr &SecondaryQueue() const; +#endif ext::oneapi::experimental::event_mode_enum &EventMode(); const ext::oneapi::experimental::event_mode_enum &EventMode() const; @@ -391,9 +393,15 @@ class __SYCL_EXPORT queue : public detail::OwnerLessBase { std::enable_if_t, event> submit( T CGF, queue &SecondaryQueue, const detail::code_location &CodeLoc = detail::code_location::current()) { - return submit_with_event<__SYCL_USE_FALLBACK_ASSERT>( - sycl::ext::oneapi::experimental::empty_properties_t{}, - detail::type_erased_cgfo_ty{CGF}, &SecondaryQueue, CodeLoc); + try { + return submit_with_event<__SYCL_USE_FALLBACK_ASSERT>( + sycl::ext::oneapi::experimental::empty_properties_t{}, + detail::type_erased_cgfo_ty{CGF}, CodeLoc); + } catch (...) { + return SecondaryQueue.submit_with_event<__SYCL_USE_FALLBACK_ASSERT>( + sycl::ext::oneapi::experimental::empty_properties_t{}, + detail::type_erased_cgfo_ty{CGF}, CodeLoc); + } } /// Prevents any commands submitted afterward to this queue from executing @@ -3519,7 +3527,7 @@ class __SYCL_EXPORT queue : public detail::OwnerLessBase { -> backend_return_t; #if __SYCL_USE_FALLBACK_ASSERT - friend event detail::submitAssertCapture(queue &, event &, queue *, + friend event detail::submitAssertCapture(queue &, event &, const detail::code_location &); #endif @@ -3603,46 +3611,6 @@ class __SYCL_EXPORT queue : public detail::OwnerLessBase { const detail::code_location &CodeLoc, bool IsTopCodeLoc); - /// Submits a command group function object to the queue, in order to be - /// scheduled for execution on the device. - /// - /// \param Props is a property list with submission properties. - /// \param CGF is a function object containing command group. - /// \param SecondaryQueuePtr is a pointer to the secondary queue. - /// \param CodeLoc is the code location of the submit call (default argument) - /// \return a SYCL event object for the submitted command group. - // - // UseFallBackAssert as template param vs `#if` in function body is necessary - // to prevent ODR-violation between TUs built with different fallback assert - // modes. - template - event submit_with_event( - PropertiesT Props, const detail::type_erased_cgfo_ty &CGF, - queue *SecondaryQueuePtr, - const detail::code_location &CodeLoc = detail::code_location::current()) { - detail::tls_code_loc_t TlsCodeLocCapture(CodeLoc); - detail::SubmissionInfo SI{}; - ProcessSubmitProperties(Props, SI); - if (SecondaryQueuePtr) - SI.SecondaryQueue() = detail::getSyclObjImpl(*SecondaryQueuePtr); - if constexpr (UseFallbackAssert) - SI.PostProcessorFunc() = - [this, &SecondaryQueuePtr, - &TlsCodeLocCapture](bool IsKernel, bool KernelUsesAssert, event &E) { - if (IsKernel && !device_has(aspect::ext_oneapi_native_assert) && - KernelUsesAssert && !device_has(aspect::accelerator)) { - // __devicelib_assert_fail isn't supported by Device-side Runtime - // Linking against fallback impl of __devicelib_assert_fail is - // performed by program manager class - // Fallback assert isn't supported for FPGA - submitAssertCapture(*this, E, SecondaryQueuePtr, - TlsCodeLocCapture.query()); - } - }; - return submit_with_event_impl(CGF, SI, TlsCodeLocCapture.query(), - TlsCodeLocCapture.isToplevel()); - } - /// Submits a command group function object to the queue, in order to be /// scheduled for execution on the device. /// @@ -3671,7 +3639,7 @@ class __SYCL_EXPORT queue : public detail::OwnerLessBase { // Linking against fallback impl of __devicelib_assert_fail is // performed by program manager class // Fallback assert isn't supported for FPGA - submitAssertCapture(*this, E, nullptr, TlsCodeLocCapture.query()); + submitAssertCapture(*this, E, TlsCodeLocCapture.query()); } }; return submit_with_event_impl(CGF, SI, TlsCodeLocCapture.query(), @@ -3870,14 +3838,13 @@ class AssertInfoCopier; * Submit copy task for assert failure flag and host-task to check the flag * \param Event kernel's event to depend on i.e. the event represents the * kernel to check for assertion failure - * \param SecondaryQueue secondary queue for submit process, null if not used * \returns host tasks event * * This method doesn't belong to queue class to overcome msvc behaviour due to * which it gets compiled and exported without any integration header and, thus, * with no proper KernelInfo instance. */ -event submitAssertCapture(queue &Self, event &Event, queue *SecondaryQueue, +event submitAssertCapture(queue &Self, event &Event, const detail::code_location &CodeLoc) { buffer Buffer{1}; @@ -3933,10 +3900,10 @@ event submitAssertCapture(queue &Self, event &Event, queue *SecondaryQueue, CopierEv = Self.submit_with_event( sycl::ext::oneapi::experimental::empty_properties_t{}, CopierCGF, - SecondaryQueue, CodeLoc); + CodeLoc); CheckerEv = Self.submit_with_event( sycl::ext::oneapi::experimental::empty_properties_t{}, CheckerCGF, - SecondaryQueue, CodeLoc); + CodeLoc); return CheckerEv; } diff --git a/sycl/source/detail/handler_impl.hpp b/sycl/source/detail/handler_impl.hpp index 42b1991f153f5..2f271f5031ce5 100644 --- a/sycl/source/detail/handler_impl.hpp +++ b/sycl/source/detail/handler_impl.hpp @@ -31,9 +31,13 @@ enum class HandlerSubmissionState : std::uint8_t { class handler_impl { public: +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES handler_impl(queue_impl *SubmissionSecondaryQueue, bool EventNeeded) : MSubmissionSecondaryQueue(SubmissionSecondaryQueue), MEventNeeded(EventNeeded) {}; +#else + handler_impl(bool EventNeeded) : MEventNeeded(EventNeeded) {}; +#endif handler_impl( std::shared_ptr Graph) @@ -67,9 +71,11 @@ class handler_impl { /// Registers mutually exclusive submission states. HandlerSubmissionState MSubmissionState = HandlerSubmissionState::NO_STATE; +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES /// Pointer to the secondary queue implementation. Nullptr if no /// secondary queue fallback was given in the associated submission. queue_impl *MSubmissionSecondaryQueue = nullptr; +#endif /// Bool stores information about whether the event resulting from the /// corresponding work is required. diff --git a/sycl/source/detail/queue_impl.cpp b/sycl/source/detail/queue_impl.cpp index 8230f3a7f4906..e668a1add5424 100644 --- a/sycl/source/detail/queue_impl.cpp +++ b/sycl/source/detail/queue_impl.cpp @@ -312,11 +312,18 @@ void queue_impl::addEvent(const event &Event) { event queue_impl::submit_impl(const detail::type_erased_cgfo_ty &CGF, const std::shared_ptr &Self, - queue_impl *SecondaryQueue, bool CallerNeedsEvent, +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES + queue_impl *SecondaryQueue, +#endif + bool CallerNeedsEvent, const detail::code_location &Loc, bool IsTopCodeLoc, const SubmissionInfo &SubmitInfo) { +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES handler Handler(Self, SecondaryQueue, CallerNeedsEvent); +#else + handler Handler(Self, CallerNeedsEvent); +#endif auto &HandlerImpl = detail::getSyclObjImpl(Handler); #ifdef XPTI_ENABLE_INSTRUMENTATION if (xptiTraceEnabled()) { @@ -352,9 +359,14 @@ event queue_impl::submit_impl(const detail::type_erased_cgfo_ty &CGF, Stream->generateFlushCommand(ServiceCGH); }; detail::type_erased_cgfo_ty CGF{L}; +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES event FlushEvent = submit_impl(CGF, Self, SecondaryQueue, /*CallerNeedsEvent*/ true, Loc, IsTopCodeLoc, {}); +#else + event FlushEvent = submit_impl(CGF, Self, /*CallerNeedsEvent*/ true, Loc, + IsTopCodeLoc, {}); +#endif EventImpl->attachEventToCompleteWeak(detail::getSyclObjImpl(FlushEvent)); registerStreamServiceEvent(detail::getSyclObjImpl(FlushEvent)); } diff --git a/sycl/source/detail/queue_impl.hpp b/sycl/source/detail/queue_impl.hpp index 0d09d05f15534..827aa3e662c0b 100644 --- a/sycl/source/detail/queue_impl.hpp +++ b/sycl/source/detail/queue_impl.hpp @@ -69,7 +69,9 @@ enum QueueOrder { Ordered, OOO }; // Implementation of the submission information storage. struct SubmissionInfoImpl { optional MPostProcessorFunc = std::nullopt; +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES std::shared_ptr MSecondaryQueue = nullptr; +#endif ext::oneapi::experimental::event_mode_enum MEventMode = ext::oneapi::experimental::event_mode_enum::none; }; @@ -339,12 +341,16 @@ class queue_impl { /// group is being enqueued on. event submit(const detail::type_erased_cgfo_ty &CGF, const std::shared_ptr &Self, +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES const std::shared_ptr &SecondQueue, +#endif const detail::code_location &Loc, bool IsTopCodeLoc, const SubmitPostProcessF *PostProcess = nullptr) { event ResEvent; SubmissionInfo SI{}; +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES SI.SecondaryQueue() = SecondQueue; +#endif if (PostProcess) SI.PostProcessorFunc() = *PostProcess; return submit_with_event(CGF, Self, SI, Loc, IsTopCodeLoc); @@ -363,10 +369,14 @@ class queue_impl { const std::shared_ptr &Self, const SubmissionInfo &SubmitInfo, const detail::code_location &Loc, bool IsTopCodeLoc) { - +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES event ResEvent = submit_impl(CGF, Self, SubmitInfo.SecondaryQueue().get(), /*CallerNeedsEvent=*/true, Loc, IsTopCodeLoc, SubmitInfo); +#else + event ResEvent = submit_impl(CGF, Self, /*CallerNeedsEvent=*/true, Loc, + IsTopCodeLoc, SubmitInfo); +#endif return discard_or_return(ResEvent); } @@ -375,8 +385,13 @@ class queue_impl { const SubmissionInfo &SubmitInfo, const detail::code_location &Loc, bool IsTopCodeLoc) { +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES submit_impl(CGF, Self, SubmitInfo.SecondaryQueue().get(), /*CallerNeedsEvent=*/false, Loc, IsTopCodeLoc, SubmitInfo); +#else + submit_impl(CGF, Self, /*CallerNeedsEvent=*/false, Loc, IsTopCodeLoc, + SubmitInfo); +#endif } /// Performs a blocking wait for the completion of all enqueued tasks in the @@ -863,9 +878,11 @@ class queue_impl { /// \return a SYCL event representing submitted command group. event submit_impl(const detail::type_erased_cgfo_ty &CGF, const std::shared_ptr &Self, - queue_impl *SecondaryQueue, bool CallerNeedsEvent, - const detail::code_location &Loc, bool IsTopCodeLoc, - const SubmissionInfo &SubmitInfo); +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES + queue_impl *SecondaryQueue, +#endif + bool CallerNeedsEvent, const detail::code_location &Loc, + bool IsTopCodeLoc, const SubmissionInfo &SubmitInfo); /// Helper function for submitting a memory operation with a handler. /// \param Self is a shared_ptr to this queue. diff --git a/sycl/source/detail/scheduler/scheduler.hpp b/sycl/source/detail/scheduler/scheduler.hpp index 5b657c1f13b93..d25a780c58e25 100644 --- a/sycl/source/detail/scheduler/scheduler.hpp +++ b/sycl/source/detail/scheduler/scheduler.hpp @@ -588,14 +588,6 @@ class Scheduler { void cleanupCommand(Command *Cmd, bool AllowUnsubmitted = false); - /// Reschedules the command passed using Queue provided. - /// - /// This can lead to rescheduling of all dependent commands. This can be - /// used when the user provides a "secondary" queue to the submit method - /// which may be used when the command fails to enqueue/execute in the - /// primary queue. - void rescheduleCommand(Command *Cmd, const QueueImplPtr &Queue); - /// \return a pointer to the corresponding memory object record for the /// SYCL memory object provided, or nullptr if it does not exist. MemObjRecord *getMemObjRecord(SYCLMemObjI *MemObject); diff --git a/sycl/source/handler.cpp b/sycl/source/handler.cpp index 46f0b4370bbb8..c8a1b0db09cfe 100644 --- a/sycl/source/handler.cpp +++ b/sycl/source/handler.cpp @@ -314,8 +314,13 @@ fill_copy_args(detail::handler_impl *impl, handler::handler(std::shared_ptr Queue, bool CallerNeedsEvent) +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES : impl(std::make_shared(nullptr, CallerNeedsEvent)), - MQueue(std::move(Queue)) {} +#else + : impl(std::make_shared(CallerNeedsEvent)), +#endif + MQueue(std::move(Queue)) { +} #ifndef __INTEL_PREVIEW_BREAKING_CHANGES // TODO: This function is not used anymore, remove it in the next @@ -327,13 +332,13 @@ handler::handler( : impl(std::make_shared(SecondaryQueue.get(), CallerNeedsEvent)), MQueue(Queue) {} -#endif handler::handler(std::shared_ptr Queue, detail::queue_impl *SecondaryQueue, bool CallerNeedsEvent) : impl(std::make_shared(SecondaryQueue, CallerNeedsEvent)), MQueue(std::move(Queue)) {} +#endif handler::handler( std::shared_ptr Graph) @@ -1775,6 +1780,7 @@ void handler::use_kernel_bundle( "Context associated with the primary queue is different from the " "context associated with the kernel bundle"); +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES if (impl->MSubmissionSecondaryQueue && impl->MSubmissionSecondaryQueue->get_context() != ExecBundle.get_context()) @@ -1782,6 +1788,7 @@ void handler::use_kernel_bundle( make_error_code(errc::invalid), "Context associated with the secondary queue is different from the " "context associated with the kernel bundle"); +#endif setStateExplicitKernelBundle(); setHandlerKernelBundle(detail::getSyclObjImpl(ExecBundle)); diff --git a/sycl/source/queue.cpp b/sycl/source/queue.cpp index 4383bb4a2d49a..b155b60777924 100644 --- a/sycl/source/queue.cpp +++ b/sycl/source/queue.cpp @@ -32,6 +32,7 @@ const optional &SubmissionInfo::PostProcessorFunc() const { return impl->MPostProcessorFunc; } +#ifndef __INTEL_PREVIEW_BREAKING_CHANGES std::shared_ptr &SubmissionInfo::SecondaryQueue() { return impl->MSecondaryQueue; } @@ -40,6 +41,7 @@ const std::shared_ptr & SubmissionInfo::SecondaryQueue() const { return impl->MSecondaryQueue; } +#endif ext::oneapi::experimental::event_mode_enum &SubmissionInfo::EventMode() { return impl->MEventMode; diff --git a/sycl/unittests/SYCL2020/KernelBundle.cpp b/sycl/unittests/SYCL2020/KernelBundle.cpp index 5ffe4ecbc341a..3de75e3393041 100644 --- a/sycl/unittests/SYCL2020/KernelBundle.cpp +++ b/sycl/unittests/SYCL2020/KernelBundle.cpp @@ -277,35 +277,33 @@ TEST(KernelBundle, UseKernelBundleWrongContextPrimaryQueueValidSecondaryQueue) { sycl::queue PrimaryQueue{PrimaryCtx, Dev}; sycl::queue SecondaryQueue{SecondaryCtx, Dev}; - class UnqiueException {}; - + size_t EnqueueCounter = 0; try { PrimaryQueue.submit( [&](sycl::handler &CGH) { try { + ++EnqueueCounter; CGH.use_kernel_bundle(KernelBundle); - FAIL() << "No exception was thrown."; + if (EnqueueCounter == 1) + FAIL() << "No exception was thrown."; CGH.single_task([]() {}); } catch (const sycl::exception &e) { + ASSERT_EQ(EnqueueCounter, size_t{1}) + << "Only the primary queue was supposed to throw."; ASSERT_EQ(e.code().value(), static_cast(sycl::errc::invalid)) << "sycl::exception code was not the expected " "sycl::errc::invalid."; - // Throw uniquely identifiable exception to distinguish between that - // the sycl::exception originates from the correct level. - throw UnqiueException{}; + throw; } catch (...) { FAIL() << "Unexpected exception was thrown in kernel invocation " "function."; } }, SecondaryQueue); - } catch (const UnqiueException &) { - // Expected path - } catch (const sycl::exception &) { - FAIL() << "sycl::exception thrown at the wrong level."; } catch (...) { FAIL() << "Unexpected exception was thrown in submit."; } + ASSERT_EQ(EnqueueCounter, size_t{2}); } TEST(KernelBundle, UseKernelBundleValidPrimaryQueueWrongContextSecondaryQueue) { @@ -323,32 +321,26 @@ TEST(KernelBundle, UseKernelBundleValidPrimaryQueueWrongContextSecondaryQueue) { sycl::queue PrimaryQueue{PrimaryCtx, Dev}; sycl::queue SecondaryQueue{SecondaryCtx, Dev}; - class UnqiueException {}; - + size_t EnqueueCounter = 0; try { PrimaryQueue.submit( [&](sycl::handler &CGH) { - try { - CGH.use_kernel_bundle(KernelBundle); - FAIL() << "No exception was thrown."; - CGH.single_task([]() {}); - } catch (const sycl::exception &e) { - ASSERT_EQ(e.code().value(), static_cast(sycl::errc::invalid)) - << "sycl::exception code was not the expected " - "sycl::errc::invalid."; - // Throw uniquely identifiable exception to distinguish between that - // the sycl::exception originates from the correct level. - throw UnqiueException{}; - } catch (...) { - FAIL() << "Unexpected exception was thrown in kernel invocation " - "function."; - } + CGH.use_kernel_bundle(KernelBundle); + ++EnqueueCounter; + // Throw a non-sycl exception to force the secondary queue to try and + // enqueue. The secondary queue should never get beyond the + // use_kernel_bundle. + throw std::exception{}; + CGH.single_task([]() {}); }, SecondaryQueue); - } catch (const UnqiueException &) { - // Expected path - } catch (const sycl::exception &) { - FAIL() << "sycl::exception thrown at the wrong level."; + FAIL() << "Submit should always throw."; + } catch (const sycl::exception &e) { + ASSERT_EQ(EnqueueCounter, size_t{1}) + << "Exception was thrown from primary queue."; + ASSERT_EQ(e.code().value(), static_cast(sycl::errc::invalid)) + << "sycl::exception code was not the expected " + "sycl::errc::invalid."; } catch (...) { FAIL() << "Unexpected exception was thrown in submit."; } @@ -372,32 +364,22 @@ TEST(KernelBundle, UseKernelBundleWrongContextPrimaryQueueAndSecondaryQueue) { sycl::queue PrimaryQueue{PrimaryCtx, Dev}; sycl::queue SecondaryQueue{SecondaryCtx, Dev}; - class UnqiueException {}; - + size_t EnqueueCounter = 0; try { PrimaryQueue.submit( [&](sycl::handler &CGH) { - try { CGH.use_kernel_bundle(KernelBundle); - FAIL() << "No exception was thrown."; + ++EnqueueCounter; CGH.single_task([]() {}); - } catch (const sycl::exception &e) { - ASSERT_EQ(e.code().value(), static_cast(sycl::errc::invalid)) - << "sycl::exception code was not the expected " - "sycl::errc::invalid."; - // Throw uniquely identifiable exception to distinguish between that - // the sycl::exception originates from the correct level. - throw UnqiueException{}; - } catch (...) { - FAIL() << "Unexpected exception was thrown in kernel invocation " - "function."; - } }, SecondaryQueue); - } catch (const UnqiueException &) { - // Expected path - } catch (const sycl::exception &) { - FAIL() << "sycl::exception thrown at the wrong level."; + FAIL() << "Submit should always throw."; + } catch (const sycl::exception &e) { + ASSERT_EQ(EnqueueCounter, size_t{0}) + << "Exception was not thrown from primary queue."; + ASSERT_EQ(e.code().value(), static_cast(sycl::errc::invalid)) + << "sycl::exception code was not the expected " + "sycl::errc::invalid."; } catch (...) { FAIL() << "Unexpected exception was thrown in submit."; } From 8464d43fd4911154a4e3623a66e71c1b05876c95 Mon Sep 17 00:00:00 2001 From: "Larsen, Steffen" Date: Sun, 1 Jun 2025 23:27:49 -0700 Subject: [PATCH 2/3] Fix faultu merge Signed-off-by: Larsen, Steffen --- sycl/include/sycl/queue.hpp | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/sycl/include/sycl/queue.hpp b/sycl/include/sycl/queue.hpp index ff9cf1a796682..36006ac341bb6 100644 --- a/sycl/include/sycl/queue.hpp +++ b/sycl/include/sycl/queue.hpp @@ -436,27 +436,18 @@ class __SYCL_EXPORT queue : public detail::OwnerLessBase { /// Submits a command group function object to the queue, in order to be /// scheduled for execution on the device. /// - /// On a kernel error, this command group function object is then scheduled - /// for execution on a secondary queue. - /// /// \param CGF is a function object containing command group. - /// \param SecondaryQueue is a fallback SYCL queue. + /// \param SecondaryQueue is a fallback SYCL queue. (unused) /// \param CodeLoc is the code location of the submit call (default argument) /// \return a SYCL event object, which corresponds to the queue the command /// group is being enqueued on. template std::enable_if_t, event> submit( - T CGF, queue &SecondaryQueue, + T CGF, [[maybe_unused]] queue &SecondaryQueue, const detail::code_location &CodeLoc = detail::code_location::current()) { - try { - return submit_with_event<__SYCL_USE_FALLBACK_ASSERT>( - sycl::ext::oneapi::experimental::empty_properties_t{}, - detail::type_erased_cgfo_ty{CGF}, CodeLoc); - } catch (...) { - return SecondaryQueue.submit_with_event<__SYCL_USE_FALLBACK_ASSERT>( - sycl::ext::oneapi::experimental::empty_properties_t{}, - detail::type_erased_cgfo_ty{CGF}, CodeLoc); - } + return submit_with_event<__SYCL_USE_FALLBACK_ASSERT>( + sycl::ext::oneapi::experimental::empty_properties_t{}, + detail::type_erased_cgfo_ty{CGF}, CodeLoc); } /// Prevents any commands submitted afterward to this queue from executing From 23b36bd6d85ea6b358d1d782ab5f6b257483520a Mon Sep 17 00:00:00 2001 From: "Larsen, Steffen" Date: Mon, 2 Jun 2025 00:08:51 -0700 Subject: [PATCH 3/3] Fix formatting and remove test Signed-off-by: Larsen, Steffen --- sycl/source/detail/queue_impl.cpp | 5 +- sycl/source/handler.cpp | 9 +-- sycl/unittests/SYCL2020/KernelBundle.cpp | 90 +----------------------- 3 files changed, 11 insertions(+), 93 deletions(-) diff --git a/sycl/source/detail/queue_impl.cpp b/sycl/source/detail/queue_impl.cpp index a94acb842782b..237b7dfb523ac 100644 --- a/sycl/source/detail/queue_impl.cpp +++ b/sycl/source/detail/queue_impl.cpp @@ -305,8 +305,9 @@ void queue_impl::addEvent(const detail::EventImplPtr &EventImpl) { } detail::EventImplPtr -queue_impl::submit_impl(const detail::type_erased_cgfo_ty &CGF, bool CallerNeedsEvent, - const detail::code_location &Loc, bool IsTopCodeLoc, +queue_impl::submit_impl(const detail::type_erased_cgfo_ty &CGF, + bool CallerNeedsEvent, const detail::code_location &Loc, + bool IsTopCodeLoc, const v1::SubmissionInfo &SubmitInfo) { #ifdef __INTEL_PREVIEW_BREAKING_CHANGES detail::handler_impl HandlerImplVal(CallerNeedsEvent); diff --git a/sycl/source/handler.cpp b/sycl/source/handler.cpp index 42f96b14ca74a..63a5c493c76a8 100644 --- a/sycl/source/handler.cpp +++ b/sycl/source/handler.cpp @@ -328,8 +328,7 @@ handler::handler(detail::handler_impl *HandlerImpl, handler::handler(std::shared_ptr Queue, bool CallerNeedsEvent) : impl(std::make_shared(CallerNeedsEvent)), - MQueue(std::move(Queue)) { -} + MQueue(std::move(Queue)) {} #ifndef __INTEL_PREVIEW_BREAKING_CHANGES // TODO: This function is not used anymore, remove it in the next @@ -337,12 +336,14 @@ handler::handler(std::shared_ptr Queue, handler::handler( std::shared_ptr Queue, [[maybe_unused]] std::shared_ptr PrimaryQueue, - [[maybe_unused]] std::shared_ptr SecondaryQueue, bool CallerNeedsEvent) + [[maybe_unused]] std::shared_ptr SecondaryQueue, + bool CallerNeedsEvent) : impl(std::make_shared(CallerNeedsEvent)), MQueue(Queue) {} handler::handler(std::shared_ptr Queue, - [[maybe_unused]] detail::queue_impl *SecondaryQueue, bool CallerNeedsEvent) + [[maybe_unused]] detail::queue_impl *SecondaryQueue, + bool CallerNeedsEvent) : impl(std::make_shared(CallerNeedsEvent)), MQueue(std::move(Queue)) {} #endif diff --git a/sycl/unittests/SYCL2020/KernelBundle.cpp b/sycl/unittests/SYCL2020/KernelBundle.cpp index 3de75e3393041..3748f002ba88c 100644 --- a/sycl/unittests/SYCL2020/KernelBundle.cpp +++ b/sycl/unittests/SYCL2020/KernelBundle.cpp @@ -262,90 +262,6 @@ TEST(KernelBundle, UseKernelBundleWrongContextPrimaryQueueOnly) { } } -TEST(KernelBundle, UseKernelBundleWrongContextPrimaryQueueValidSecondaryQueue) { - sycl::unittest::UrMock<> Mock; - - const sycl::device Dev = sycl::platform().get_devices()[0]; - const sycl::context PrimaryCtx{Dev}; - const sycl::context SecondaryCtx{Dev}; - - ASSERT_NE(PrimaryCtx, SecondaryCtx); - - auto KernelBundle = sycl::get_kernel_bundle( - SecondaryCtx, {Dev}); - - sycl::queue PrimaryQueue{PrimaryCtx, Dev}; - sycl::queue SecondaryQueue{SecondaryCtx, Dev}; - - size_t EnqueueCounter = 0; - try { - PrimaryQueue.submit( - [&](sycl::handler &CGH) { - try { - ++EnqueueCounter; - CGH.use_kernel_bundle(KernelBundle); - if (EnqueueCounter == 1) - FAIL() << "No exception was thrown."; - CGH.single_task([]() {}); - } catch (const sycl::exception &e) { - ASSERT_EQ(EnqueueCounter, size_t{1}) - << "Only the primary queue was supposed to throw."; - ASSERT_EQ(e.code().value(), static_cast(sycl::errc::invalid)) - << "sycl::exception code was not the expected " - "sycl::errc::invalid."; - throw; - } catch (...) { - FAIL() << "Unexpected exception was thrown in kernel invocation " - "function."; - } - }, - SecondaryQueue); - } catch (...) { - FAIL() << "Unexpected exception was thrown in submit."; - } - ASSERT_EQ(EnqueueCounter, size_t{2}); -} - -TEST(KernelBundle, UseKernelBundleValidPrimaryQueueWrongContextSecondaryQueue) { - sycl::unittest::UrMock<> Mock; - - const sycl::device Dev = sycl::platform().get_devices()[0]; - const sycl::context PrimaryCtx{Dev}; - const sycl::context SecondaryCtx{Dev}; - - ASSERT_NE(PrimaryCtx, SecondaryCtx); - - auto KernelBundle = sycl::get_kernel_bundle( - PrimaryCtx, {Dev}); - - sycl::queue PrimaryQueue{PrimaryCtx, Dev}; - sycl::queue SecondaryQueue{SecondaryCtx, Dev}; - - size_t EnqueueCounter = 0; - try { - PrimaryQueue.submit( - [&](sycl::handler &CGH) { - CGH.use_kernel_bundle(KernelBundle); - ++EnqueueCounter; - // Throw a non-sycl exception to force the secondary queue to try and - // enqueue. The secondary queue should never get beyond the - // use_kernel_bundle. - throw std::exception{}; - CGH.single_task([]() {}); - }, - SecondaryQueue); - FAIL() << "Submit should always throw."; - } catch (const sycl::exception &e) { - ASSERT_EQ(EnqueueCounter, size_t{1}) - << "Exception was thrown from primary queue."; - ASSERT_EQ(e.code().value(), static_cast(sycl::errc::invalid)) - << "sycl::exception code was not the expected " - "sycl::errc::invalid."; - } catch (...) { - FAIL() << "Unexpected exception was thrown in submit."; - } -} - TEST(KernelBundle, UseKernelBundleWrongContextPrimaryQueueAndSecondaryQueue) { sycl::unittest::UrMock<> Mock; @@ -368,9 +284,9 @@ TEST(KernelBundle, UseKernelBundleWrongContextPrimaryQueueAndSecondaryQueue) { try { PrimaryQueue.submit( [&](sycl::handler &CGH) { - CGH.use_kernel_bundle(KernelBundle); - ++EnqueueCounter; - CGH.single_task([]() {}); + CGH.use_kernel_bundle(KernelBundle); + ++EnqueueCounter; + CGH.single_task([]() {}); }, SecondaryQueue); FAIL() << "Submit should always throw.";