Skip to content

[libspirv] Don't force fp16/fp64 OpenCL extensions #18961

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
Jun 16, 2025

Conversation

frasercrmck
Copy link
Contributor

Enabling these (or any extensions, really) by default is incorrect and prevents the r600 OpenCL builtins from being built. Each clang target reports which OpenCL extensions it does or doesn't support.

This commit also moves the flag enabling SPIR-V builtin declarations to the SPIR-V options. They are not needed for the compilation of CLC or OpenCL builtins.

There is no change to any builtins file.

Enabling these (or any extensions, really) by default is incorrect and
prevents the r600 OpenCL builtins from being built. Each clang target
reports which OpenCL extensions it does or doesn't support.

There is no change to any builtins file.
@frasercrmck frasercrmck requested a review from a team as a code owner June 12, 2025 16:09
@frasercrmck frasercrmck requested a review from ldrumm June 12, 2025 16:09
@frasercrmck frasercrmck requested a review from wenju-he June 12, 2025 16:10
Copy link
Contributor

@npmiller npmiller left a comment

Choose a reason for hiding this comment

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

This looks fine, provided the extensions do get properly enabled depending on the target.

Note that r600 is a very old target, so we don't really care about it for SYCL support in general, but this is a small change and a cleanup for other targets as well so it's fine.

@frasercrmck
Copy link
Contributor Author

This looks fine, provided the extensions do get properly enabled depending on the target.

Note that r600 is a very old target, so we don't really care about it for SYCL support in general, but this is a small change and a cleanup for other targets as well so it's fine.

The r600 target doesn't actually get built for libspirv anymore - a recent change made libspirv only build for three supported targets. The error was just that we were unconditionally enabling these OpenCL extensions for both SPIR-V and OpenCL. But since we've also enabled the 3D image writes extensions in the NVPTX clang target downstream, we can't not specify them here. It's all a bit backwards. Note that @wenju-he has an upstream PR to try and fix the 3D image writes problem: llvm/llvm-project#143331.

There's no change to any of the builtin libraries with this PR.

Copy link
Contributor

@wenju-he wenju-he left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @frasercrmck

@frasercrmck
Copy link
Contributor Author

@intel/llvm-gatekeepers this is also ready to merge, thank you

@steffenlarsen steffenlarsen merged commit 3ffee67 into intel:sycl Jun 16, 2025
25 checks passed
@frasercrmck frasercrmck deleted the libspirv-no-khr-fpx branch June 16, 2025 09:13
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