Skip to content

[SYCL][UR][Bindless][L0] Fix cherry-pick of linear mem interop patch #19193

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

Open
wants to merge 1 commit into
base: sycl-rel-6_2
Choose a base branch
from

Conversation

przemektmalon
Copy link
Contributor

This patch fixes the bad cherry pick of #18353.

@przemektmalon przemektmalon marked this pull request as ready for review June 27, 2025 10:39
@przemektmalon przemektmalon requested a review from a team as a code owner June 27, 2025 10:39
@przemektmalon
Copy link
Contributor Author

FYI @AlexeySachkov @KornevNikita, this patch fixes the bad cherry pick of #18353.

I've tested this on Linux on a BMG GPU and the linear memory interop test passes, although if you could also validate this independently that would be very helpful.

The relevant test is sycl/test-e2e/bindless_images/vulkan_interop/buffer_usm.cpp (you may or may not have issues running the other Vulkan interop tests on Level Zero, if so, these would not be related to this PR).

Copy link
Contributor

@KornevNikita KornevNikita left a comment

Choose a reason for hiding this comment

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

@przemektmalon do you plan to cherry-pick something else in the scope of this PR? If no, please push the branch to intel/llvm, so I can launch a workflow on this branch.
Saying in advance - ignore the pre-commit, it's broken.

@bjoernknafla
Copy link
Contributor

bjoernknafla commented Jun 27, 2025

@KornevNikita The related code from #18353 is already in intel/llvm. However, when the related commit cf18fe1 was cherry-picked into the release branch some changes were not applied cleanly as intel/llvm already contained non-release commits that are not in the release branch.

This PR is a patch of the release branch for the code that is missing from the cherry-pick.

Do we need to approach the fix for the release branch differently?

@KornevNikita
Copy link
Contributor

@bjoernknafla @przemektmalon yep I got it. To launch the proper validation I need this branch to be in intel/llvm, not in your fork. Please push it.

@bjoernknafla
Copy link
Contributor

@KornevNikita Do you need a PR of the fix to trigger pre-commit testing or do you need it to be merged?

Asking as that will complicate the patch to handle differences between intel/llvm and the release branch, i.e., part of the functionality has moved to a file that does not exist on the release branch.

@KornevNikita
Copy link
Contributor

The branch is codeplaysoftware:przemek/fix-bi-cherry-pick-linear-mem-interop
We need it to be in intel/llvm, i.e. intel:przemek/fix-bi-cherry-pick-linear-mem-interop
I need you to do this:

git remote add intel_llvm https://github.com/intel/llvm.git
git push --set-upstream intel_llvm

Is it possible for you?

@bjoernknafla
Copy link
Contributor

bjoernknafla commented Jun 27, 2025

@KornevNikita My understanding is:

  • You do not need the PR to target or be merged into intel/llvm.
  • Instead, you need us to move the source branch of the PR from codeplaysoftware:przemek/fix-bi-cherry-pick-linear-mem-interop to intel/llvm. The PR would still target intel/sycl-rel-6_2.

We will need to try to see if we have the necessary permissions (after lunch).

@KornevNikita
Copy link
Contributor

@bjoernknafla yes, that's what I'm trying to say. If you can't do it, please ping me, I'll try myself.

@przemektmalon
Copy link
Contributor Author

@KornevNikita I've created a PR from intel/llvm here: #19194

@KornevNikita
Copy link
Contributor

@przemektmalon thanks! Actually there is no need for new PR if there is no difference between them. Anyway, I launched the validation - https://github.com/intel/llvm/actions/runs/15927114994. If the result is fine - then we can merge this one.

@bjoernknafla
Copy link
Contributor

@KornevNikita Thank you for your patience and explaining the process to me - I misunderstood the meaning of "push" and then overcomplicated what you said in my head.

@KornevNikita
Copy link
Contributor

@bjoernknafla sure, no problem.

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.

3 participants