Repository navigation
Conversation
mmichel11
changed the title
enqueue_signal_event and enqueue_wait_event[s]enqueue_signal_event and enqueue_wait_event[s]
There was a problem hiding this comment.
🟡 Changes recommended
Removing the legacy public declarations breaks source compatibility for callers taking function addresses.
1 open finding
What changed in this PR
Adds property-list support to reusable event enqueue APIs while preserving existing ABI symbols.
Changes:
- Adds constrained property-list overloads and internal flag plumbing.
- Retains legacy runtime entry points.
- Adds compile-time, unit, and ABI coverage.
| File | Description |
|---|---|
sycl/include/sycl/ext/oneapi/experimental/reusable_events.hpp |
Defines property-list enqueue APIs. |
sycl/source/reusable_events.cpp |
Implements flag-based entry points and ABI wrappers. |
sycl/unittests/Extensions/ReusableEvents.cpp |
Tests empty property lists. |
sycl/test/extensions/reusable_events_constraints.cpp |
Tests property constraints. |
sycl/test/abi/sycl_symbols_linux.dump |
Records Linux ABI symbols. |
sycl/test/abi/sycl_symbols_windows.dump |
Records Windows ABI symbols. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| template <typename PropertyListT = empty_properties_t, | ||
| typename = std::enable_if_t<detail::all_are_properties_of_v< | ||
| detail::enqueue_event_props, PropertyListT>>> | ||
| void enqueue_wait_event(sycl::queue q, const event &evt, | ||
| PropertyListT props = {}) { | ||
| detail::enqueue_wait_event(q, evt, detail::getEnqueueEventFlags(props)); | ||
| } |
36c70d4 to
aa8aa98
Compare
|
|
||
| } // namespace detail | ||
|
|
||
| // These entry points are kept for backwards compatibility. |
There was a problem hiding this comment.
@dm-vodopyanov @intel/llvm-reviewers-runtime Could you advise if this is the best approach to preserve ABI?
We had discussed with @gmlueck that the libsycl ABI should be maintained here. The enqueue_* functions used to be exported directly. They are now header templates that take a property list and call new detail:: exports with a flags argument. I removed the previous declarations from the header but kept these old exported symbols, so binaries built against older headers still work.
Is it fine that calls with no provided property list call into the new detail:: functions?
enqueue_signal_event,enqueue_wait_event, andenqueue_wait_eventsdefined in [SYCL][Doc] Propose a graph external event extension to reusable events #23377make_event