-
Notifications
You must be signed in to change notification settings - Fork 787
[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
base: sycl-rel-6_2
Are you sure you want to change the base?
[SYCL][UR][Bindless][L0] Fix cherry-pick of linear mem interop patch #19193
Conversation
This patch fixes the bad cherry pick of intel#18353.
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 |
There was a problem hiding this 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.
@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? |
@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. |
@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. |
The branch is
Is it possible for you? |
@KornevNikita My understanding is:
We will need to try to see if we have the necessary permissions (after lunch). |
@bjoernknafla yes, that's what I'm trying to say. If you can't do it, please ping me, I'll try myself. |
@KornevNikita I've created a PR from |
@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. |
@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. |
@bjoernknafla sure, no problem. |
This patch fixes the bad cherry pick of #18353.