oneDPL icon indicating copy to clipboard operation
oneDPL copied to clipboard

[tests] Add Input Data Sweep tests

Open danhoeflinger opened this issue 1 year ago • 1 comments

Note: Should be merged after #1445 and #1438, as errors can occur otherwise.

Summary: Adding test coverage to catch bugs which slipped through our previous testing by targeting a sweep of interesting combinations of input data. Would have detected the following bugs which were missed by our test coverage at the time:

#1413 (f69618b4e) reverse_iterator: detects as a runtime failure. #1371 (286097b43) zip_iterator device copyable: detects as a build error #1383 (a860b5d1b) 3 fixes for permutation iterator: detects as build error #1341 (fef095835) "general case" with recursion of permutation iterator: detects as build error #1325 (6d50dbad9) broken bracket operator permutation iterator: detects as build error


Details: The test does not specifically target these bugs listed above, but rather identifies the target base data, wrappers, and recurses to generate a large number of possible iterator types. It runs these resulting iterator types through a minimal kernel (copy of 10 elements) as both a read and write and checks the result. Some types and wrappers disable tests or aspects of the resulting tests for deeper levels of recursion.

Base Types:

  1. float (recurse 0)
  2. double (recurse 0)
  3. uint64_t (recurse 0)
  4. int32_t (recurse 2 layers of wrappers)

Sequence types:

  1. host_iterator
  2. sycl_iterator
  3. counting_iterator (for integral types)
  4. usm_shared
  5. usm_device
  6. std::vector<T,usm_allocator<T,sycl::usm::alloc::shared>>::iterator
  7. std::vector<T,usm_allocator<T,sycl::usm::alloc::host>>::iterator

Wrappers:

  1. std::reverse_iterator(it)
  2. transform_iterator(it, noop{})
  3. permutation_iteartor(it,noop{})
  4. permutation_iterator(it, counting_iter)
  5. permutation_iterator(counting_iterator, it)
  6. zip_iterator(counting_iterator, it)
  7. zip_iterator(it, discard_iterator)

It also explores a couple special cases each sent through 1 level of recursion:

  1. discard_iterators
  2. permutation_iterator( permutation_iterator( usm_shared<int>, counting_iterator), counting_iterator)

This PR catches 2 issues

  1. build error for zip_iterator<it, transform_iterator<it,lambda> inputs, which is fixed by #1445.

  2. Inefficient and potentially incorrect handling of std::vector<T,usm_allocator>::iterator. This has been addressed by #1438. Note: Depending on implementation of std::vector, some std::vector<T>::iterator cannot be distinguished from std::vector<T, usm_allocator>::iterator. For those cases, we must continue handling these types as if they were host_iterators.

Note: I've disabled some combinations which don't make sense including std::reverse_iterator(sycl_iterator based "iterators") because sycl_iterators are not iterators. In practice, those do seem to work on linux, but not windows. There are build errors on windows when attempting to run the SFINAE trick for __is_addressable.

danhoeflinger avatar Mar 01 '24 21:03 danhoeflinger

I've split this into individual tests for each input sequence type (host_iter, sycl_iterator, usm_device, usm_shared, counting_iter). When grouped together, there is some potential for running out of memory during compilation in some environments.

danhoeflinger avatar Mar 06 '24 13:03 danhoeflinger

Could you rebase this branch with main, so we can ensure everything works well after the merge of PR #1445? I'm ready to approve after.

mmichel11 avatar Apr 18 '24 13:04 mmichel11

Could you rebase this branch with main, so we can ensure everything works well after the merge of PR #1445? I'm ready to approve after.

Good point... done.

danhoeflinger avatar Apr 18 '24 13:04 danhoeflinger