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] Update return types for image extent queries #16829

Conversation

przemektmalon
Copy link
Contributor

Updated return types of max_image_linear_{dim/pitch} queries to match the Bindless Images extension specification.

While uint32_t is more than sufficient in most situations, CUDA uses size_t values for these properties when creating textures. Vulkan also allows implementations to use sizes greater than what can be expressed by uint32_t.

Updated return types of `max_image_linear_{dim/pitch}` to match the
Bindless Images extension specification.

While `uint32_t` is more than sufficient in most situations, CUDA uses
`size_t` values for these properties when creating textures. Vulkan
also allows implementations to use sizes greater than what can be
expressed by `uint32_t`.
@przemektmalon
Copy link
Contributor Author

I'm assuming here that the ABI changes fall under the experimental extension exception, and are permissible to merge.

Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Some of the symbols here are just moved, likely due to the person adding them not using the tool for generating the symbols. Only ABI breaking changes here seem to be experimental, so they should be good.

@przemektmalon
Copy link
Contributor Author

@intel/llvm-gatekeepers I believe this can be merged.

Copy link
Contributor

@aelovikov-intel aelovikov-intel left a comment

Choose a reason for hiding this comment

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

Some of the symbols here are just moved, likely due to the person adding them not using the tool for generating the symbols. Only ABI breaking changes here seem to be experimental, so they should be good.

That's wrong approach. Either re-generated in a separate PR prior to making your change, or manually drop unrelated changes.

@aelovikov-intel aelovikov-intel dismissed their stale review January 29, 2025 16:47

Ok, agree after taking more time to look at this.

@aelovikov-intel aelovikov-intel merged commit 0e5b5ee into intel:sycl Jan 29, 2025
17 checks passed
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.

4 participants