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.

Sign up for GitHub

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

Jump to bottom

[L0] Refactoring of boolean event parameters #1600

Merged
merged 5 commits into from
Feb 11, 2025
Merged

[L0] Refactoring of boolean event parameters #1600

merged 5 commits into from
Feb 11, 2025

Conversation

winstonzhang-intel
Copy link
Contributor

@winstonzhang-intel winstonzhang-intel commented May 14, 2024 •
edited
Loading

CI in LLVM/SYCL: intel/llvm#14536

@github-actions github-actions bot added the level-zero L0 adapter specific issues label May 14, 2024
@winstonzhang-intel winstonzhang-intel force-pushed the event-bool-refactoring branch 2 times, most recently from eafcd64 to 9679ead Compare May 20, 2024 15:26
@winstonzhang-intel winstonzhang-intel marked this pull request as ready for review May 21, 2024 16:20
@winstonzhang-intel winstonzhang-intel requested a review from a team as a code owner May 21, 2024 16:20
nrspruit
nrspruit previously requested changes May 24, 2024
View reviewed changes
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.

Looks good, I found one typo in the flag checks, please fix that and verify that there are not others I missed, thanks!

pbalcer
pbalcer reviewed May 27, 2024
View reviewed changes
source/adapters/level_zero/context.hpp Outdated Show resolved Hide resolved
source/adapters/level_zero/common.hpp Outdated Show resolved Hide resolved
source/adapters/level_zero/context.cpp Outdated Show resolved Hide resolved
@nrspruit nrspruit self-requested a review July 1, 2024 20:14
@nrspruit nrspruit dismissed their stale review July 1, 2024 20:14

no longer relevant.

@nrspruit
Copy link
Contributor

nrspruit commented Jul 3, 2024

Hello @winstonzhang-intel ,

Please rebase since there are L0 CTS failures here: https://github.com/oneapi-src/unified-runtime/actions/runs/9651142091/job/26618341665?pr=1600.

@winstonzhang-intel winstonzhang-intel force-pushed the event-bool-refactoring branch 2 times, most recently from 38b0489 to e1d74f2 Compare July 11, 2024 14:29
winstonzhang-intel added a commit to winstonzhang-intel/llvm that referenced this pull request Jul 11, 2024
@winstonzhang-intel
[UR] Refactoring of boolean event parameters
5539104 
PR in URT: oneapi-src/unified-runtime#1600

Signed-off-by: Winston Zhang <[email protected]>
@winstonzhang-intel winstonzhang-intel mentioned this pull request Jul 11, 2024
Draft
@pbalcer pbalcer force-pushed the event-bool-refactoring branch from e1d74f2 to dd16ed7 Compare July 24, 2024 15:57
@pbalcer pbalcer mentioned this pull request Jul 24, 2024
Merged
winstonzhang-intel added a commit to winstonzhang-intel/llvm that referenced this pull request Sep 10, 2024
@winstonzhang-intel
[UR] Refactoring of boolean event parameters
e81de92 
PR in URT: oneapi-src/unified-runtime#1600

Signed-off-by: Winston Zhang <[email protected]>
nrspruit
nrspruit approved these changes Sep 10, 2024
View reviewed changes
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

@nrspruit nrspruit added the ready to merge Added to PR's which are ready to merge label Sep 13, 2024
@omarahmed1111
Copy link
Contributor

@winstonzhang-intel Please make sure that intel/llvm#14536 is updated and passing CI, so we could merge this. Thanks!

@omarahmed1111 omarahmed1111 removed the ready to merge Added to PR's which are ready to merge label Sep 18, 2024
@omarahmed1111
Copy link
Contributor

I removed the ready to merge label temporarily until the intel/llvm side is updated and passing tests. Please readd it when intel/llvm PR is updated.

winstonzhang-intel added a commit to winstonzhang-intel/llvm that referenced this pull request Sep 18, 2024
@winstonzhang-intel
[UR] Refactoring of boolean event parameters
af7498b 
PR in URT: oneapi-src/unified-runtime#1600

Signed-off-by: Winston Zhang <[email protected]>
winstonzhang-intel added a commit to winstonzhang-intel/llvm that referenced this pull request Sep 18, 2024
@winstonzhang-intel
[UR] Refactoring of boolean event parameters
20efe20 
PR in URT: oneapi-src/unified-runtime#1600

Signed-off-by: Winston Zhang <[email protected]>
@winstonzhang-intel
Copy link
Contributor Author

I removed the ready to merge label temporarily until the intel/llvm side is updated and passing tests. Please readd it when intel/llvm PR is updated.

I removed the ready to merge label temporarily until the intel/llvm side is updated and passing tests. Please readd it when intel/llvm PR is updated.

Working on rebasing against latest phase 2 of counterbased patch.

winstonzhang-intel added a commit to winstonzhang-intel/llvm that referenced this pull request Sep 19, 2024
@winstonzhang-intel
[UR] Refactoring of boolean event parameters
b2bc5b5 
PR in URT: oneapi-src/unified-runtime#1600

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

nrspruit commented Dec 2, 2024

@winstonzhang-intel , please resolve the merge conflicts with a rebase otherwise the changes look good.

@nrspruit nrspruit mentioned this pull request Dec 2, 2024
Merged
@winstonzhang-intel winstonzhang-intel force-pushed the event-bool-refactoring branch 2 times, most recently from 2cae6db to 0f1de75 Compare December 19, 2024 02:12
@winstonzhang-intel
[L0] Refactor of boolean variables for Event
d99434c 
This now includes the interrupt changes and is rebased on top of
getEventCache changes.

Signed-off-by: Zhang, Winston <[email protected]>
@winstonzhang-intel
[L0] further simplify context.hpp
1a130dc 
Signed-off-by: Zhang, Winston <[email protected]>
@winstonzhang-intel
[L0] Consolidated event_flags into v2/event_provider.hpp
7fad387 
Signed-off-by: Zhang, Winston <[email protected]>
@winstonzhang-intel
[L0] Further simplified getZeEventPoolCache
0ba88a3 
Signed-off-by: Zhang, Winston <[email protected]>
@winstonzhang-intel
[L0] reverted getEventCache due to test failure
f1c14d4 
Signed-off-by: Zhang, Winston <[email protected]>
@nrspruit nrspruit added the ready to merge Added to PR's which are ready to merge label Feb 3, 2025
@kbenzie kbenzie merged commit b05ffb5 into oneapi-src:main Feb 11, 2025
26 checks passed
@kbenzie kbenzie mentioned this pull request Feb 12, 2025
Merged
@kbenzie
Copy link
Contributor

kbenzie commented Feb 12, 2025

@winstonzhang-intel unfortunately I had to revert this due to the CI failures on intel/llvm#14536, they are very confusing to me.

@kbenzie
Copy link
Contributor

kbenzie commented Feb 12, 2025

This could be related to intel/llvm#16747 (comment) which I think should have been resolved by #2690 but will need to investigate further.

@winstonzhang-intel
Copy link
Contributor Author

This could be related to intel/llvm#16747 (comment) which I think should have been resolved by #2690 but will need to investigate further.

Sounds good, please let me know. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pbalcer pbalcer pbalcer left review comments

@nrspruit nrspruit nrspruit approved these changes

@raiyanla raiyanla Awaiting requested review from raiyanla

@igchor igchor Awaiting requested review from igchor

Assignees
No one assigned
Labels
level-zero L0 adapter specific issues ready to merge Added to PR's which are ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@winstonzhang-intel @nrspruit @omarahmed1111 @kbenzie @pbalcer