Skip to content

Commit

Permalink
[SYCL][ESIMD] Fix a few issues with scatter(usm, ...) (#12585)
Browse files Browse the repository at this point in the history
Problems found by Gregory (thanks!):

1) There were some duplicated tests, remove those

2) We didn't test non-LSC mask on Gen12

3) We get an ambiguous call because we had an old function that didn't
have VS, but the new functions have default VS=1, so we don't need the
old one.

4) When we pass a simd_view for the vals, we got a template match
failure. This is the same issue we hit in the compile-time tests where
even if we have a simd_view overload the compiler can't infer N, so we
need to provide T,N anyway, so add that in the tests.

I tested this on Gen12.

Signed-off-by: Sarnie, Nick <[email protected]>
  • Loading branch information
sarnex authored Feb 1, 2024
1 parent 0dc97ec commit 8bfc56f
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 25 deletions.
18 changes: 0 additions & 18 deletions sycl/include/sycl/ext/intel/esimd/memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -854,24 +854,6 @@ scatter(T *p, OffsetSimdViewT byte_offsets, simd<T, N> vals,
scatter<T, N, VS>(p, byte_offsets.read(), vals, Mask, props);
}

/// A variation of \c scatter API with \c offsets represented as \c simd_view
/// object.
///
/// @tparam Tx Element type, must be of size 4 or less.
/// @tparam N Number of elements to write; can be \c 1, \c 2, \c 4, \c 8, \c 16
/// or \c 32.
/// @param p The base address.
/// @param offsets A simd_view of 32-bit or 64-bit offsets in bytes. For each
/// lane \c i, ((byte*)p + offsets[i]) must be element size aligned.
/// @param vals The vector to scatter.
/// @param mask The access mask, defaults to all 1s.
///
template <typename Tx, int N, typename OffsetObjT, typename RegionTy>
__ESIMD_API void scatter(Tx *p, simd_view<OffsetObjT, RegionTy> offsets,
simd<Tx, N> vals, simd_mask<N> mask = 1) {
scatter<Tx, N, 1>(p, offsets.read(), vals, mask);
}

/// A variation of \c scatter API with \c offsets represented as scalar.
///
/// @tparam Tx Element type, must be of size 4 or less.
Expand Down
19 changes: 12 additions & 7 deletions sycl/test-e2e/ESIMD/unified_memory_api/Inputs/scatter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ bool testUSM(queue Q, uint32_t MaskStride,
else if (GlobalID % 4 == 1)
scatter(Out, ByteOffsetsView, Vals, Pred);
else if (GlobalID % 4 == 2)
scatter(Out, ByteOffsets, ValsView, Pred);
scatter<T, N>(Out, ByteOffsets, ValsView, Pred);
else if (GlobalID % 4 == 3)
scatter(Out, ByteOffsetsView, ValsView, Pred);
scatter<T, N>(Out, ByteOffsetsView, ValsView, Pred);
}
} else { // UseMask == false
if constexpr (UseProperties) {
Expand Down Expand Up @@ -182,20 +182,25 @@ template <typename T, TestFeatures Features> bool testUSM(queue Q) {

bool Passed = true;

// // Test scatter() that is available on Gen12 and PVC.
// Test scatter() that is available on Gen12 and PVC.
Passed &= testUSM<T, 1, 1, !CheckMask, CheckProperties>(Q, 2, EmptyProps);
Passed &= testUSM<T, 2, 1, !CheckMask, CheckProperties>(Q, 1, EmptyProps);
Passed &= testUSM<T, 1, 1, !CheckMask, CheckProperties>(Q, 2, EmptyProps);
Passed &= testUSM<T, 4, 1, !CheckMask, CheckProperties>(Q, 2, EmptyProps);
Passed &= testUSM<T, 8, 1, !CheckMask, CheckProperties>(Q, 2, EmptyProps);
Passed &= testUSM<T, 16, 1, !CheckMask, CheckProperties>(Q, 2, EmptyProps);
Passed &= testUSM<T, 16, 1, !CheckMask, CheckProperties>(Q, 2, EmptyProps);

Passed &= testUSM<T, 32, 1, !CheckMask, CheckProperties>(Q, 2, EmptyProps);

// // Test scatter() without passing compile-time properties argument.
// Test scatter() without passing compile-time properties argument.
Passed &= testUSM<T, 16, 1, !CheckMask, !CheckProperties>(Q, 2, EmptyProps);
Passed &= testUSM<T, 32, 1, !CheckMask, !CheckProperties>(Q, 2, EmptyProps);

// Test scatter() with mask
Passed &= testUSM<T, 2, 1, CheckMask, CheckProperties>(Q, 2, EmptyProps);
Passed &= testUSM<T, 4, 1, CheckMask, CheckProperties>(Q, 2, EmptyProps);
Passed &= testUSM<T, 8, 1, CheckMask, CheckProperties>(Q, 2, EmptyProps);
Passed &= testUSM<T, 16, 1, CheckMask, !CheckProperties>(Q, 2, EmptyProps);

if constexpr (Features == TestFeatures::PVC ||
Features == TestFeatures::DG2) {
properties LSCProps{cache_hint_L1<cache_hint::streaming>,
Expand All @@ -219,7 +224,7 @@ template <typename T, TestFeatures Features> bool testUSM(queue Q) {
Passed &=
testUSM<T, 32, 2, CheckMask, CheckProperties>(Q, 2, AlignElemProps);
Passed &=
testUSM<T, 32, 2, CheckMask, CheckProperties>(Q, 2, AlignElemProps);
testUSM<T, 32, 2, CheckMask, !CheckProperties>(Q, 2, AlignElemProps);
}
} // TestPVCFeatures

Expand Down

0 comments on commit 8bfc56f

Please sign in to comment.