Skip to content
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

Disallow repeated swizzle element in assignment #585

Merged

Conversation

gmlueck
Copy link
Contributor

@gmlueck gmlueck commented Jul 17, 2024

We previously decided that assignment to a __writeable_swizzle__ should have a constraint that the lhs does not have any repeated swizzle elements. Thus, something like this is disallowed:

vec<int, 2> x;
myvec.swizzle<1,1>() = x;

We did this because the result would depend on the order in which the assignments occurred, and we think that some compilers will not handle this case well.

However, we neglected to add the same constraint to other assignment-like swizzle operations. This commit adds a similar constraint, which disallows all of the following cases also, where the __writeable_swizzle__ has repeated elements:

myvec.swizzle<1,1>().load(off, p);
myvec.swizzle<1,1>() += x;
myvec.swizzle<1,1>()++;
++(myvec.swizzle<1,1>());

We previously decided that assignment to a `__writeable_swizzle__`
should have a constraint that the lhs does not have any repeated
swizzle elements.  Thus, something like this is disallowed:

```
vec<int, 2> x;
myvec.swizzle<1,1>() = x;
```

We did this because the result would depend on the order in which the
assignments occurred, and we think that some compilers will not handle
this case well.

However, we neglected to add the same constraint to other
assignment-like swizzle operations.  This commit adds a similar
constraint, which disallows all of the following cases also, where
the `__writeable_swizzle__` has repeated elements:

```
myvec.swizzle<1,1>().load(off, p);
myvec.swizzle<1,1>() += x;
myvec.swizzle<1,1>()++;
++(myvec.swizzle<1,1>());
```
Copy link
Collaborator

@AerialMantis AerialMantis left a comment

Choose a reason for hiding this comment

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

Good catch, looks good to me.

@gmlueck gmlueck added the Agenda To be discussed during a SYCL committee meeting label Jul 18, 2024
@tomdeakin tomdeakin merged commit 0bba4db into KhronosGroup:SYCL-2020/master Jul 25, 2024
2 checks passed
@gmlueck gmlueck deleted the gmlueck/swizzle-no-repeat branch July 25, 2024 20:17
keryell pushed a commit that referenced this pull request Sep 10, 2024
Disallow repeated swizzle element in assignment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agenda To be discussed during a SYCL committee meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants