Skip to content

Add sycl::convert free function as scalar equivalent of vec::convert#995

Open
ssam18 wants to merge 2 commits intoKhronosGroup:mainfrom
ssam18:fix/issue-981-scalar-convert
Open

Add sycl::convert free function as scalar equivalent of vec::convert#995
ssam18 wants to merge 2 commits intoKhronosGroup:mainfrom
ssam18:fix/issue-981-scalar-convert

Conversation

@ssam18
Copy link
Copy Markdown

@ssam18 ssam18 commented Apr 4, 2026

sycl::vec::convert lets you convert between element types using explicit rounding modes, but there was no way to do the same thing for plain scalar values. This PR adds a sycl::convert<ConvertT, RoundingMode>(value) free function that fills that gap. The rounding modes section and its table caption are also updated to reflect that they now apply to both scalars and vectors.

vec::convert supports explicit rounding modes but there was no way to
do the same for scalar types. This adds a sycl::convert<ConvertT,
RoundingMode>(value) free function and documents it in a new section
alongside the updated rounding modes table.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 4, 2026

CLA assistant check
All committers have signed the CLA.

@ssam18
Copy link
Copy Markdown
Author

ssam18 commented Apr 4, 2026

I have signed the Contribution License Agreement, Still CI/CD workflow is pending on that. When I click, it shows you already have signed. Not sure how to fix it.

@ssam18 ssam18 closed this Apr 4, 2026
@ssam18 ssam18 reopened this Apr 4, 2026
@TApplencourt
Copy link
Copy Markdown
Contributor

Thanks for your contribution.

Why did you choose a free function ? It conflict with many the other API.

You should explain why you choose this implementation, and the other alternative you didn't consider.

Also just for your know, it's kind of bad practice to not discuss first the issue inside the ticket and opening a PR first. The goal is too reach consensus on the fix before implementing it.


PS: In the case of LLM assistance, please review what Claude did wrote as justification before posting. The burden of reviewing the output should be on you, and not on the committee.

@ssam18
Copy link
Copy Markdown
Author

ssam18 commented Apr 7, 2026

@TApplencourt
Fair points, thank you. On process, I should have discussed the approach in the issue before opening the PR and I will be doing that going forward.

On the design choice, I went with a free function to give scalars a clean equivalent of vec::convert without forcing users to wrap/unwrap a vec<T,1>. The main alternatives I considered were:

  1. vec<T,1>::convert --> already works but is verbose and requires the user to treat a scalar as a single-element vector
  2. static_cast --> no rounding mode control
  3. A convert_cast<T, RM>(value) free function --> follows SYCL's existing address_space_cast naming convention and avoids the generic convert name

In my opinion convert_cast is probably a better name. It is more consistent with existing SYCL conventions and avoids any collision concerns. Happy to rename if the group agrees that's the right direction.

PS: I have been writing C++ for over 20 years, shipped high-performance systems at ABB, Intel, Cisco, Johnson Controls, and currently work as a Principal Engineer at HP leading Edge AI projects. I reject Claude's suggestions 60% of the time. The spec comment and this PR were mine to own, that's on me for not discussing in the issue first, not on the tool.

@TApplencourt
Copy link
Copy Markdown
Contributor

TApplencourt commented Apr 9, 2026

@ssam18
Copy link
Copy Markdown
Author

ssam18 commented Apr 13, 2026

I see your point. I reworked this as a KHR extension instead and moved the function under sycl::khr::convert, added the feature test macro, and pulled it out of the core spec entirely.

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.

3 participants