Skip to content

Commit

Permalink
Merge pull request #683 from aleksmesh/fix_wrong_checking_for_placeho…
Browse files Browse the repository at this point in the history
…lder_constructors

Fix placeholder accessor wrong checking
  • Loading branch information
bader authored Jun 28, 2023
2 parents 552133a + 3d739d5 commit 9552084
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 20 deletions.
66 changes: 59 additions & 7 deletions tests/accessor/accessor_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,8 @@ struct tag_factory<accessor_type::host_accessor> {
* @param testing_acc Instance of TestingAccT that were constructed with default
* constructor
* @param res_acc Instance of result accessor
* @param check_iterator_methods Flag to avoid undefined behavior on access to
* uninitialized underlying buffer
*/
template <typename TestingAccT, typename ResultAccT>
void check_empty_accessor_constructor_post_conditions(
Expand Down Expand Up @@ -391,11 +393,57 @@ void check_def_constructor(GetAccFunctorT get_accessor_functor) {

auto acc = get_accessor_functor();
if constexpr (AccType != accessor_type::host_accessor) {
// Disable checking iteration methods with empty device accessor
// to avoid undefined behavior
bool check_iterator_methods = false;
check_empty_accessor_constructor_post_conditions(acc, conditions_check,
false);
check_iterator_methods);
} else {
bool check_iterator_methods = true;
check_empty_accessor_constructor_post_conditions(acc, conditions_check,
true);
check_iterator_methods);
}

for (size_t i = 0; i < conditions_checks_size; i++) {
CHECK(conditions_check[i]);
}
}

/**
* @brief Common function that constructs placeholder accessor with zero-length
* buffer and checks post-conditions
*
* @tparam AccType Type of the accessor
* @tparam DataT Type of underlying data
* @tparam Dimension Dimensions of the accessor
* @tparam AccessMode Access mode of the accessor
* @tparam Target Target of accessor
* @tparam GetAccFunctorT Type of functor for accessor creation
*/
template <accessor_type AccType, typename DataT, int Dimension,
sycl::access_mode AccessMode = sycl::access_mode::read_write,
sycl::target Target = sycl::target::device, typename GetAccFunctorT>
void check_zero_length_buffer_placeholder_constructor(
GetAccFunctorT get_accessor_functor) {
auto queue = once_per_unit::get_queue();
constexpr int buf_dims = (0 == Dimension) ? 1 : Dimension;
auto r = util::get_cts_object::range<buf_dims>::get(0, 0, 0);
sycl::buffer<DataT, buf_dims> data_buf(r);
const size_t conditions_checks_size = 8;
bool conditions_check[conditions_checks_size];
std::fill(conditions_check, conditions_check + conditions_checks_size, true);

auto acc = get_accessor_functor(data_buf);
if constexpr (AccType != accessor_type::host_accessor) {
// Disable checking iteration methods with empty device accessor
// to avoid undefined behavior
bool check_iterator_methods = false;
check_empty_accessor_constructor_post_conditions(acc, conditions_check,
check_iterator_methods);
} else {
bool check_iterator_methods = true;
check_empty_accessor_constructor_post_conditions(acc, conditions_check,
check_iterator_methods);
}

for (size_t i = 0; i < conditions_checks_size; i++) {
Expand Down Expand Up @@ -437,23 +485,27 @@ void check_zero_length_buffer_constructor(GetAccFunctorT get_accessor_functor) {
sycl::accessor<bool, 1, sycl::access_mode::read_write, Target>
res_acc(res_buf, cgh);
auto acc = get_accessor_functor(data_buf, cgh);
// Disable checking iteration methods with empty device accessor
// to avoid undefined behavior
bool check_iterator_methods = false;
if constexpr (Target == sycl::target::host_task) {
cgh.host_task([=] {
check_empty_accessor_constructor_post_conditions(acc, res_acc,
false);
check_empty_accessor_constructor_post_conditions(
acc, res_acc, check_iterator_methods);
});
} else if constexpr (Target == sycl::target::device) {
cgh.parallel_for_work_group(r, [=](sycl::group<Dimension>) {
check_empty_accessor_constructor_post_conditions(acc, res_acc,
false);
check_empty_accessor_constructor_post_conditions(
acc, res_acc, check_iterator_methods);
});
}
})
.wait_and_throw();
} else {
auto acc = get_accessor_functor(data_buf);
bool check_iterator_methods = true;
check_empty_accessor_constructor_post_conditions(acc, conditions_check,
true);
check_iterator_methods);
}

for (size_t i = 0; i < conditions_checks_size; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ void test_placeholder_zero_length_buffer_constructor(

SECTION(section_name) {
constexpr int dim_buf = (0 == Dimension) ? 1 : Dimension;
auto get_acc_functor = [](sycl::buffer<DataT, dim_buf>& data_buf,
sycl::handler& cgh) {
auto get_acc_functor = [](sycl::buffer<DataT, dim_buf>& data_buf) {
return sycl::accessor<DataT, Dimension, AccessMode, Target>(data_buf);
};
check_zero_length_buffer_constructor<AccType, DataT, Dimension, AccessMode,
Target>(get_acc_functor);
check_zero_length_buffer_placeholder_constructor<AccType, DataT, Dimension,
AccessMode, Target>(
get_acc_functor);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ void test_placeholder_zero_length_buffer_range_constructor(
"From zero-length buffer and range placeholder constructor");

SECTION(section_name) {
auto get_acc_functor = [r_zero](sycl::buffer<DataT, Dimension>& data_buf,
sycl::handler& cgh) {
auto get_acc_functor = [r_zero](sycl::buffer<DataT, Dimension>& data_buf) {
return sycl::accessor<DataT, Dimension, AccessMode, Target>(data_buf,
r_zero);
};
check_zero_length_buffer_constructor<AccType, DataT, Dimension, AccessMode,
Target>(get_acc_functor);
check_zero_length_buffer_placeholder_constructor<AccType, DataT, Dimension,
AccessMode, Target>(
get_acc_functor);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ void test_placeholder_zero_length_buffser_range_offset_constructor(
"From zero-length buffer, range and offset placeholder constructor");

SECTION(section_name) {
auto get_acc_functor = [r_zero, offset](
sycl::buffer<DataT, Dimension>& data_buf,
sycl::handler& cgh) {
auto get_acc_functor = [r_zero,
offset](sycl::buffer<DataT, Dimension>& data_buf) {
return sycl::accessor<DataT, Dimension, AccessMode, Target>(
data_buf, r_zero, offset);
};
check_zero_length_buffer_constructor<AccType, DataT, Dimension, AccessMode,
Target>(get_acc_functor);
check_zero_length_buffer_placeholder_constructor<AccType, DataT, Dimension,
AccessMode, Target>(
get_acc_functor);
}
}

Expand Down

0 comments on commit 9552084

Please sign in to comment.