Skip to content

[SYCL][NativeCPU] Fix __clc_exp10. #17403

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 17, 2025
Merged

[SYCL][NativeCPU] Fix __clc_exp10. #17403

merged 1 commit into from
Mar 17, 2025

Conversation

hvdijk
Copy link
Contributor

@hvdijk hvdijk commented Mar 12, 2025

In the libspirv builtins, no definitions of fma and ldexp are provided, and there must be no references to them. Call __spirv_ocl_fma and __spirv_ocl_ldexp instead.

@hvdijk
[SYCL][NativeCPU] Fix __clc_exp10.
4ab1cfd 
In the libspirv builtins, no definitions of fma and ldexp are provided,
and there must be no references to them. Call __spirv_ocl_fma and
__spirv_ocl_ldexp instead.
@hvdijk hvdijk requested a review from a team as a code owner March 12, 2025 09:23
@hvdijk hvdijk requested a review from omarahmed1111 March 12, 2025 09:23
@hvdijk hvdijk temporarily deployed to WindowsCILock March 12, 2025 09:24 — with GitHub Actions Inactive
@hvdijk
Copy link
Contributor Author

hvdijk commented Mar 12, 2025

cc @jsji, I think this was a bad pulldown in #16781 that went unnoticed because most targets have their own exp10 definition rather than using this generic one, before that PR this file was using __spirv_ocl_* functions?

@hvdijk hvdijk temporarily deployed to WindowsCILock March 12, 2025 09:38 — with GitHub Actions Inactive
@hvdijk hvdijk temporarily deployed to WindowsCILock March 14, 2025 15:24 — with GitHub Actions Inactive
frasercrmck
frasercrmck approved these changes Mar 17, 2025
View reviewed changes
Copy link
Contributor

@frasercrmck frasercrmck left a comment

Choose a reason for hiding this comment

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

LGTM.

It does look like a pulldown issue. Upstream, and at the next pulldown, these will be __clc_fma and __clc_ldexp. But until then we should stick with the SPIR-V versions.

@hvdijk
Copy link
Contributor Author

hvdijk commented Mar 17, 2025

@intel/llvm-gatekeepers This is ready to be merged, thanks.

@martygrant martygrant merged commit f85999b into intel:sycl Mar 17, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frasercrmck frasercrmck frasercrmck approved these changes

@omarahmed1111 omarahmed1111 Awaiting requested review from omarahmed1111 omarahmed1111 is a code owner automatically assigned from intel/llvm-reviewers-cuda

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hvdijk @frasercrmck @martygrant