Skip to content

Conversation

gmlueck
Copy link
Contributor

@gmlueck gmlueck commented Sep 21, 2023

The uniform extension was implemented in #5871. Move the extension
specification document to the "experimental" directory, and update to the
latest specification template.

Also fix a small bug in the implementation. The sycl::id type should
not be disallowed for use in uniform. Although, it is invalid to
pass the work-item's ID to uniform (because that value is not uniform
across all work-items), the user could construct their own id that
does have a uniform value.

The uniform extension was implemented in intel#5871.  Move the extension
specification document to the "experimental" directory.
This makes sense and matches the implementation.
@gmlueck gmlueck requested a review from a team as a code owner September 21, 2023 20:52
@gmlueck
Copy link
Contributor Author

gmlueck commented Sep 21, 2023

There is one small substantive change. See c90080b.

We decided that `sycl::id` should not be a disallowed uniform type.
It not allowed to pass a work-item's ID to `uniform` because that
value does not have a uniform value for all work-items.  However,
the application can construct its own `sycl::id` in a uniform way,
so we should not disallow `sycl::id` in general.

This commit restores the spec wording and also changes the
implementation to align with that wording.
@gmlueck gmlueck requested a review from a team as a code owner September 22, 2023 17:30
@gmlueck gmlueck temporarily deployed to WindowsCILock September 22, 2023 17:32 — with GitHub Actions Inactive
@gmlueck gmlueck temporarily deployed to WindowsCILock September 22, 2023 18:06 — with GitHub Actions Inactive
Copy link
Contributor

@KseniyaTikhomirova KseniyaTikhomirova left a comment

Choose a reason for hiding this comment

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

LGTM

@bader bader merged commit 7796057 into intel:sycl Sep 27, 2023
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.

4 participants