[SYCL][ESIMD] Add simd::copy* E2E tests. #236
Conversation
Signed-off-by: kbobrovs <[email protected]>
|
|
||
| std::cout << (passed ? "=== Test passed\n" : "=== Test FAILED\n"); | ||
| return passed ? 0 : 1; | ||
| } |
There was a problem hiding this comment.
nit: would it become more readable if we just split USM- and accessor-based tests into separate files? Some if constexpr would go away.
There was a problem hiding this comment.
Yes, it might. But I'd prefer to have this in a single file to avoid code duplication and reduce maintenance burden.
| int main(int argc, char **argv) { | ||
| size_t size = 32 * 7; | ||
|
|
||
| if (argc > 1) { |
There was a problem hiding this comment.
Do we need that code? Is it left intentionally for debugging purposes?
There was a problem hiding this comment.
it is left intentionally, to be able to run with different size.
| } | ||
|
|
||
| int main(int argc, char **argv) { | ||
| size_t size = 32 * 7; |
There was a problem hiding this comment.
Is it intentionally 32 * 7, not 32 << 7 ?
| const uint32_t ii = static_cast<uint32_t>(i.get(0)); | ||
| simd<T, N> v; | ||
| const auto offset = ii * sizeof(v); | ||
| v.copy_from(acc, offset); |
There was a problem hiding this comment.
We copy the source data into the original destination. If the copy_from and copy_to would be unimplemented the test will pass as well. Maybe we can test introduce a separate destination buffer and test copying A -> B?
There was a problem hiding this comment.
Actually, not quite. The test does:
v.copy_from(ptr + offset);
v += simd<T, N>(ii * N, 1);
v.copy_to(ptr + offset);I.e. the written value depends both on the original value. So if either does not work test should fail. But the choice of '0' as the initial value is bad indeed, as there is a chance v can be initialized to zeroes even if copy_from does not work. I'll fix that.
There was a problem hiding this comment.
I mean, the point is still good :), as it lead to problem discovery, but it is not fully valid.
|
@kbobrovs, could you address comments? |
Yes, this somehow went off the radar. |
Signed-off-by: kbobrovs <[email protected]>
|
@kbobrovs should we modify namespaces? They have been changed in compiler... |
|
@kbobrovs could you please fix CI failures? |
@vladimirlaz - fixed. sorry for delay |
Signed-off-by: kbobrovs <[email protected]>
Signed-off-by: kbobrovs <[email protected]>
|
@vladimirlaz, this one is ready to merge. |
Signed-off-by: kbobrovs <[email protected]>
Signed-off-by: kbobrovs <[email protected]>
Tests for new ESIMD API, see intel/llvm#3572
Signed-off-by: kbobrovs [email protected]