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][Bindless][Exp] Fix get_mip_level_mem_handle declaration. #12822

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

tangjj11
Copy link
Contributor

A declaration of get_mip_level_mem_handle incorrectly took sycl::device as a parameter, which should have been a sycl::queue. This is now fixed.

A declaration of get_mip_level_mem_handle incorrectly took sycl::device as a parameter, which should have been a sycl::queue. This is now fixed.
@isaacault
Copy link
Contributor

Is this not an ABI break? I would've expected CI to catch that and fail..

I'd think that the right way to do this would be to deprecate this definition and export the new, correct, definition, although with CI passing I'm not sure what to think. Perhaps we should deprecate anyway, to be proper? Or am I misunderstanding?

@tangjj11
Copy link
Contributor Author

Is this not an ABI break? I would've expected CI to catch that and fail..

I'd think that the right way to do this would be to deprecate this definition and export the new, correct, definition, although with CI passing I'm not sure what to think. Perhaps we should deprecate anyway, to be proper? Or am I misunderstanding?

I thinks that the reason why the CI has not catch the ABI breaking fail is the same as @przemektmalon's reply in #11948.

@tangjj11
Copy link
Contributor Author

@intel/llvm-gatekeepers can this be merged? thanks

@sommerlukas sommerlukas merged commit 9edd27a into intel:sycl Feb 27, 2024
10 checks passed
@tangjj11 tangjj11 deleted the get-mip-level-param-bug branch February 27, 2024 09:51
@sommerlukas
Copy link
Contributor

Unrelated test failures:

  • Unexpected passing test USM/usm_leak_check.cpp
  • Failure in basic_tests/SYCL-2020-spec-constants.cpp
  • sycl-ls failure on Windows

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.

5 participants