Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[L0] Interrupt-based event implementation #2334

Merged
merged 12 commits into from
Dec 5, 2024

Conversation

winstonzhang-intel
Copy link
Contributor

@winstonzhang-intel winstonzhang-intel commented Nov 16, 2024

To expose this functionality in UR, we want two ways of enabling low-power events:

  1. Queue-wide enabling so all events created on the queue are low-powered.
  2. As a property passed to urEnqueueEventsWaitWithBarrier making the resulting event a low-power event. This will require the existing interface to be extended with properties, potentially through a new experimental function.

SYCL/LLVM: intel/llvm#16252

@github-actions github-actions bot added level-zero L0 adapter specific issues command-buffer Command Buffer feature addition/changes/specification labels Nov 16, 2024
@winstonzhang-intel winstonzhang-intel force-pushed the interrupt-based branch 2 times, most recently from 7835bc8 to 28e7d38 Compare November 22, 2024 02:31
@winstonzhang-intel winstonzhang-intel marked this pull request as ready for review November 22, 2024 02:55
@winstonzhang-intel winstonzhang-intel requested review from a team as code owners November 22, 2024 02:55
source/adapters/level_zero/event.cpp Outdated Show resolved Hide resolved
source/adapters/level_zero/event.cpp Outdated Show resolved Hide resolved
source/adapters/level_zero/context.cpp Outdated Show resolved Hide resolved
source/adapters/level_zero/context.hpp Show resolved Hide resolved
@nrspruit nrspruit added the v0.11.x Include in the v0.11.x release label Dec 1, 2024
Copy link
Contributor

@EwanC EwanC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

command_buffer.cpp LGTM

To expose this functionality in UR, we want two ways of enabling low-power events:

Queue-wide enabling so all events created on the queue are low-powered.
As a property passed to urEnqueueEventsWaitWithBarrier making the resulting event a low-power event. This will require the existing interface to be extended with properties, potentially through a new experimental function.

Signed-off-by: Zhang, Winston <[email protected]>
Signed-off-by: Zhang, Winston <[email protected]>
@winstonzhang-intel winstonzhang-intel force-pushed the interrupt-based branch 2 times, most recently from 379f325 to fe798d6 Compare December 3, 2024 18:07
…eueEventsWaitWithBarrierExt

Signed-off-by: Zhang, Winston <[email protected]>
Copy link
Contributor

@nrspruit nrspruit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for the updates.

Signed-off-by: Zhang, Winston <[email protected]>
Copy link
Contributor

@nrspruit nrspruit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the syntax fixes.

Copy link
Contributor

@nrspruit nrspruit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is failing reporting no devices sporadically. It appears unrelated.

@nrspruit nrspruit added the ready to merge Added to PR's which are ready to merge label Dec 3, 2024
@martygrant martygrant merged commit 5f4a5a2 into oneapi-src:main Dec 5, 2024
73 checks passed
martygrant added a commit to winstonzhang-intel/llvm that referenced this pull request Dec 5, 2024
martygrant added a commit to intel/llvm that referenced this pull request Dec 5, 2024
URT PR: oneapi-src/unified-runtime#2334

---------

Signed-off-by: Zhang, Winston <[email protected]>
Co-authored-by: Martin Morrison-Grant <[email protected]>
eventSyncMode.syncModeFlags =
ZE_INTEL_EVENT_SYNC_MODE_EXP_FLAG_LOW_POWER_WAIT |
ZE_INTEL_EVENT_SYNC_MODE_EXP_FLAG_SIGNAL_INTERRUPT;
ZeEventPoolDesc.pNext = &eventSyncMode;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! This stores a pointer to a temporary in ZeEventPoolDesc.pNext, and also potentially (with CounterBasedEventEnabled == true) overwrites the previously stored value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command-buffer Command Buffer feature addition/changes/specification level-zero L0 adapter specific issues ready to merge Added to PR's which are ready to merge v0.11.x Include in the v0.11.x release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants