Skip to content

Conversation

@fineg74
Copy link
Contributor

@fineg74 fineg74 commented May 29, 2024

No description provided.

@fineg74 fineg74 temporarily deployed to WindowsCILock May 29, 2024 16:32 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to WindowsCILock May 29, 2024 17:46 — with GitHub Actions Inactive
@fineg74 fineg74 marked this pull request as ready for review May 29, 2024 19:14
@fineg74 fineg74 requested a review from a team as a code owner May 29, 2024 19:14
/// This function is identical to (lacc-ga-1) except that the \p byte_offsets
/// is represented as \c simd_view.
template <
int VS, typename T, int N, typename AccessorT, typename OffsetSimdViewT,
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to compute N here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case no, since it has pass_thru parameter of simd<T,N> type and can deduct N from there

Copy link
Contributor

Choose a reason for hiding this comment

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

I think in such cases it is useful to have a static_assert verifying that N from pass_thru and N from simd_view object are identical.
It would be much friendlier than seeing a long list consisting of 1 error + long list of notes explaining why the call could not be resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added static_asserts where possible

/// simd<T, N> gather(AccessorT acc, OffsetSimdViewT byte_offsets,
/// simd_mask<N / VS> mask, PassThruSimdViewT pass_thru,
/// PropertyListT props = {});
/// This function is identical to (lacc-ga-1) except that the \p byte_offsets
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this comment isn't accurate for a few of the overloads

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed some comments

@fineg74 fineg74 temporarily deployed to WindowsCILock May 30, 2024 20:53 — with GitHub Actions Inactive
simd<T, N>>
gather(AccessorT acc, OffsetSimdViewT byte_offsets, simd_mask<N / VS> mask,
simd<T, N> pass_thru, PropertyListT props = {}) {
static_assert(N / VS ==
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to make an internal tracker to add these asserts to the APIs we already changed

@fineg74 fineg74 temporarily deployed to WindowsCILock May 30, 2024 21:17 — with GitHub Actions Inactive
@sarnex sarnex merged commit 43f6332 into intel:sycl May 31, 2024
@fineg74 fineg74 deleted the simdViewGatherAcc branch May 31, 2024 18:38
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