Fix swizzle-to-vec conversion #800
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We discovered one other problem with the `vec` specification during
implementation. The conversion operator from a swizzle to a `vec` with
the same number of elements currently has a constraint that disables
the conversion for a 1-element swizzle. This prevents code like this
from compiling:
```
void foo(sycl::vec<int, 1>) {}
int main() {
sycl::vec<int, 2> v;
foo(v.swizzle<1>());
}
```
I think this slipped through the cracks because at one point we were
thinking that the variadic `vec` constructor would handle this
conversion. However, we ended up adding a constraint to that variadic
constructor which disables it for 1-element parameters. As a result
the variadic constructor does not handle this case.
The CTS already tests this case, so the CTS will not pass in its
current form unless an implementation removes the constraint as shown
in this PR.
This was referenced Apr 17, 2025
TApplencourt
approved these changes
Apr 24, 2025
Contributor
Author
|
WG approved to merge |
gmlueck
added a commit
to gmlueck/SYCL-Docs
that referenced
this pull request
Apr 24, 2025
Cherry pick KhronosGroup#800 from main (cherry picked from commit 9b72a4b)
aelovikov-intel
added a commit
to intel/llvm
that referenced
this pull request
Apr 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We discovered one other problem with the
vecspecification during implementation. The conversion operator from a swizzle to avecwith the same number of elements currently has a constraint that disables the conversion for a 1-element swizzle. This prevents code like this from compiling:I think this slipped through the cracks because at one point we were thinking that the variadic
vecconstructor would handle this conversion. However, we ended up adding a constraint to that variadic constructor which disables it for 1-element parameters. As a result the variadic constructor does not handle this case.The CTS already tests this case, so the CTS will not pass in its current form unless an implementation removes the constraint as shown in this PR.