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

[SYCL][P2P] Fix info query for P2P #15873

Merged
merged 7 commits into from
Oct 31, 2024
Merged

Conversation

JackAKirk
Copy link
Contributor

@JackAKirk JackAKirk commented Oct 25, 2024

There has been some confusion I think originating in the fact that L0 backend returned bool type instead of int for P2P queries. This issue is fixed here oneapi-src/unified-runtime#2246

This PR correspondingly updates the dpc++ runtime.

Fixes #15841

@JackAKirk JackAKirk requested review from a team as code owners October 25, 2024 13:48
@JackAKirk JackAKirk requested a review from bso-intel October 25, 2024 13:48
@JackAKirk JackAKirk changed the title [sycl][P2P] Fix info query for P2P [SYCL][P2P] Fix info query for P2P Oct 25, 2024
@JackAKirk
Copy link
Contributor Author

JackAKirk commented Oct 29, 2024

@bso-intel Could you take a quick look at this so that we can merge these fixes?

Cmake will be updated when oneapi-src/unified-runtime#2246 is merged, so no need to worry about that.

Thanks

@pbalcer
Copy link
Contributor

pbalcer commented Oct 29, 2024

I merged the UR PR, please update the tags.

@JackAKirk
Copy link
Contributor Author

I merged the UR PR, please update the tags.

Thanks, done.

@pbalcer
Copy link
Contributor

pbalcer commented Oct 30, 2024

@intel/llvm-reviewers-runtime please review, this is blocking UR updates.

Copy link
Contributor

@uditagarwal97 uditagarwal97 left a comment

Choose a reason for hiding this comment

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

LGTM, just one minor suggestion.

sycl/source/device.cpp Outdated Show resolved Hide resolved
@JackAKirk
Copy link
Contributor Author

All reviewers comments have been resolved and the UR tag is already updated. @intel/llvm-gatekeepers this is ready to merge.
Thanks

@sommerlukas sommerlukas merged commit e08b06f into intel:sycl Oct 31, 2024
13 checks passed
@sarnex
Copy link
Contributor

sarnex commented Oct 31, 2024

@JackAKirk Could this HIP postcommit failure be related?

Failed Tests (1):
  SYCL :: Graph/Explicit/event_status_querying.cpp

https://github.com/intel/llvm/actions/runs/11612687147/job/32337271582

@JackAKirk
Copy link
Contributor Author

@JackAKirk Could this HIP postcommit failure be related?

Failed Tests (1):
  SYCL :: Graph/Explicit/event_status_querying.cpp

https://github.com/intel/llvm/actions/runs/11612687147/job/32337271582

From looking at that Graph test and its includes I don't see how this PR could be related to that failure.
If there was not a recent PR that affects that code then this looks likely to be another flaky failure as a result of having a non officially supported ROCM card in the CI:

We have raised this issue several times both internally to line-management (and above) as well as via public issues. Here are a few examples I quickly found:

#7634 (comment)
#12997 (comment)
#14404
#14947

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P2P Access Not Working on Level Zero
5 participants