Skip to content

[UR] Update some naming inconsistencies for UR enums #17406

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

Merged
merged 1 commit into from
Mar 24, 2025

Conversation

martygrant
Copy link
Contributor

@martygrant martygrant commented Mar 12, 2025

  • Renamed X_SUPPORTED enums to be consistent with majority of UR enums named X_SUPPORT
  • Renamed some enums to be called X_SUPPORT where they return true/false
  • Renamed SYCL_PI_CUDA_ENABLE_IMAGE_SUPPORT to SYCL_UR_CUDA_ENABLE_IMAGE_SUPPORT

@martygrant martygrant force-pushed the martin/namingConsistency branch from e5fdd0a to e41b561 Compare March 13, 2025 10:49
@martygrant martygrant force-pushed the martin/namingConsistency branch from e41b561 to d804ef2 Compare March 13, 2025 10:54
@martygrant martygrant force-pushed the martin/namingConsistency branch from d804ef2 to 85d6914 Compare March 13, 2025 11:09
@martygrant martygrant force-pushed the martin/namingConsistency branch from 85d6914 to 5971de5 Compare March 17, 2025 11:44
@martygrant martygrant force-pushed the martin/namingConsistency branch from 5971de5 to 35f1388 Compare March 17, 2025 11:59
@martygrant martygrant force-pushed the martin/namingConsistency branch from 35f1388 to 0b5115b Compare March 17, 2025 14:57
@martygrant martygrant marked this pull request as ready for review March 17, 2025 17:48
@martygrant martygrant requested review from a team as code owners March 17, 2025 17:48
Copy link
Contributor

@AlexeySachkov AlexeySachkov left a comment

Choose a reason for hiding this comment

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

sycl/doc changes LGTM. This is an interface/API break to some extent, but I think that it is fine to have it, because the variable is explicitly documented under "debugging" section with a disclaimer that it should not be used in production code. As such, I think that our right to change it without prior notice is reserved for us.

If we want to make the transition smoother, then I would suggest to print a warning message to stderr if the old variable is used

@martygrant martygrant force-pushed the martin/namingConsistency branch from 0b5115b to e44b6b6 Compare March 20, 2025 15:20
@martygrant
Copy link
Contributor Author

@intel/bindless-images-reviewers @intel/dpcpp-nativecpu-reviewers @intel/llvm-reviewers-runtime friendly ping for a review please

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.

SYCL changes LGTM

@martygrant martygrant force-pushed the martin/namingConsistency branch from e44b6b6 to 424f0d5 Compare March 21, 2025 11:38
…ENABLE_IMAGE_SUPPORT env variable, SYCL_UR_CUDA_ENABLE_IMAGE_SUPPORT should be used instead.
@martygrant martygrant force-pushed the martin/namingConsistency branch from 424f0d5 to fecd0c0 Compare March 21, 2025 17:00
@martygrant martygrant merged commit 4337f0f into intel:sycl Mar 24, 2025
31 checks passed
KornevNikita pushed a commit that referenced this pull request May 27, 2025
- Renamed `X_SUPPORTED` enums to be consistent with majority of UR enums
named `X_SUPPORT`
- Renamed some enums to be called `X_SUPPORT` where they return
true/false
- Renamed `SYCL_PI_CUDA_ENABLE_IMAGE_SUPPORT` to
`SYCL_UR_CUDA_ENABLE_IMAGE_SUPPORT`
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.

8 participants