Add sycl::convert free function as scalar equivalent of vec::convert#995
Add sycl::convert free function as scalar equivalent of vec::convert#995ssam18 wants to merge 2 commits intoKhronosGroup:mainfrom
Conversation
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.
|
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. |
|
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. |
|
@TApplencourt 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:
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. |
|
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. |
sycl::vec::convertlets 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 asycl::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.