Skip to content

Commit

Permalink
Add a single instance for creating buffer lables
Browse files Browse the repository at this point in the history
  • Loading branch information
GagaLP committed Oct 9, 2023
1 parent cde3587 commit eeebacb
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 41 deletions.
10 changes: 7 additions & 3 deletions include/buffer_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,13 @@ namespace detail {
m_buffer_infos.at(bid).debug_name = debug_name;
}

std::string get_debug_name(const buffer_id bid) const {
std::lock_guard lock(m_mutex);
return m_buffer_infos.at(bid).debug_name;
std::string get_debug_label(const buffer_id bid) const {
std::string name;
{
std::lock_guard lock(m_mutex);
name = m_buffer_infos.at(bid).debug_name;
}
return !name.empty() ? fmt::format("B{} \"{}\"", bid, name) : fmt::format("B{}", bid);
}

private:
Expand Down
14 changes: 3 additions & 11 deletions src/print_graph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,25 +34,17 @@ const char* task_type_string(const task_type tt) {
}
}

std::string get_buffer_label(const buffer_id bid, const std::string& name = "") {
// if there is no name defined, the name will be the buffer id.
// if there is a name we want "id name"
return !name.empty() ? fmt::format("B{} \"{}\"", bid, name) : fmt::format("B{}", bid);
}

void format_requirements(std::string& label, const reduction_list& reductions, const access_list& accesses, const side_effect_map& side_effects,
const access_mode reduction_init_mode) {
for(const auto& [rid, bid, buffer_name, init_from_buffer] : reductions) {
auto rmode = init_from_buffer ? reduction_init_mode : cl::sycl::access::mode::discard_write;
const region scalar_region(box<3>({0, 0, 0}, {1, 1, 1}));
const std::string bl = get_buffer_label(bid, buffer_name);
fmt::format_to(std::back_inserter(label), "<br/>(R{}) <i>{}</i> {} {}", rid, detail::access::mode_traits::name(rmode), bl, scalar_region);
fmt::format_to(std::back_inserter(label), "<br/>(R{}) <i>{}</i> {} {}", rid, detail::access::mode_traits::name(rmode), buffer_name, scalar_region);
}

for(const auto& [bid, buffer_name, mode, req] : accesses) {
const std::string bl = get_buffer_label(bid, buffer_name);
// While uncommon, we do support chunks that don't require access to a particular buffer at all.
if(!req.empty()) { fmt::format_to(std::back_inserter(label), "<br/><i>{}</i> {} {}", detail::access::mode_traits::name(mode), bl, req); }
if(!req.empty()) { fmt::format_to(std::back_inserter(label), "<br/><i>{}</i> {} {}", detail::access::mode_traits::name(mode), buffer_name, req); }
}

for(const auto& [hoid, order] : side_effects) {
Expand Down Expand Up @@ -102,7 +94,7 @@ std::string get_command_label(const node_id local_nid, const command_record& cmd
auto add_reduction_id_if_reduction = [&]() {
if(cmd.reduction_id.has_value() && cmd.reduction_id != 0) { fmt::format_to(std::back_inserter(label), "(R{}) ", cmd.reduction_id.value()); }
};
const std::string buffer_label = cmd.buffer_id.has_value() ? get_buffer_label(cmd.buffer_id.value(), cmd.buffer_name) : "";
const std::string buffer_label = cmd.buffer_id.has_value() ? cmd.buffer_name : "";

switch(cmd.type) {
case command_type::epoch: {
Expand Down
4 changes: 2 additions & 2 deletions src/recorders.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace celerity::detail {
// Naming

std::string get_buffer_name(const buffer_id bid, const buffer_manager* buff_man) {
return buff_man != nullptr ? buff_man->get_buffer_info(bid).debug_name : "";
return buff_man != nullptr ? buff_man->get_debug_label(bid) : fmt::format("B{}", bid);
}

// Tasks
Expand Down Expand Up @@ -91,7 +91,7 @@ std::optional<buffer_id> get_buffer_id(const abstract_command& cmd) {
}

std::string get_cmd_buffer_name(const std::optional<buffer_id>& bid, const buffer_manager* buff_mngr) {
if(buff_mngr == nullptr || !bid.has_value()) return "";
if(!bid.has_value()) return "";
return get_buffer_name(bid.value(), buff_mngr);
}

Expand Down
8 changes: 2 additions & 6 deletions src/worker_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,8 @@ namespace detail {
const auto acc_sr = access_map.get_requirements_for_nth_access(i, tsk->get_dimensions(), data.sr, tsk->get_global_size()).get_subrange();
const auto oob_sr = subrange<3>(oob_min, range_cast<3>(oob_max - oob_min));
const auto buffer_id = access_map.get_nth_access(i).first;
const auto buffer_name = m_buffer_mngr.get_debug_name(buffer_id);
CELERITY_ERROR("Out-of-bounds access in host task detected: Accessor {} for buffer {} attempted to access indices between {} which are "
"outside of mapped subrange {}",
i, (buffer_name.empty() ? fmt::format("{}", buffer_id) : buffer_name), oob_sr, acc_sr);
"outside of mapped subrange {}", i, m_buffer_mngr.get_debug_label(buffer_id), oob_sr, acc_sr);
}
}
#endif
Expand Down Expand Up @@ -301,10 +299,8 @@ namespace detail {
const auto acc_sr = access_map.get_requirements_for_nth_access(i, tsk->get_dimensions(), data.sr, tsk->get_global_size()).get_subrange();
const auto oob_sr = subrange<3>(oob_min, range_cast<3>(oob_max - oob_min));
const auto buffer_id = access_map.get_nth_access(i).first;
const auto buffer_name = m_buffer_mngr.get_debug_name(buffer_id);
CELERITY_ERROR("Out-of-bounds access in kernel '{}' detected: Accessor {} for buffer {} attempted to access indices between {} which are "
"outside of mapped subrange {}",
tsk->get_debug_name(), i, (buffer_name.empty() ? fmt::format("{}", buffer_id) : buffer_name), oob_sr, acc_sr);
"outside of mapped subrange {}", tsk->get_debug_name(), i, m_buffer_mngr.get_debug_label(buffer_id), oob_sr, acc_sr);
}
sycl::free(m_oob_indices_per_accessor[i], m_queue.get_sycl_queue());
}
Expand Down
28 changes: 10 additions & 18 deletions test/accessor_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -686,18 +686,13 @@ namespace detail {
q.slow_full_sync();
}

const auto attempted_sr =
subrange<3>{id_cast<3>(oob_idx_lo), range_cast<3>(oob_idx_hi - oob_idx_lo + id_cast<Dims>(range<Dims>(ones))) - range_cast<3>(range<Dims>(zeros))};
const auto unnamed_error_message =
fmt::format("Out-of-bounds access in kernel 'celerity::detail::acc_out_of_bounds_kernel<{}>' detected: Accessor 0 for buffer 0 attempted to "
"access indices between {} which are outside of mapped subrange {}",
Dims, attempted_sr, subrange_cast<3>(accessible_sr));
const auto attempted_sr = subrange<3>{id_cast<3>(oob_idx_lo), range_cast<3>(oob_idx_hi - oob_idx_lo + id_cast<Dims>(range<Dims>(ones))) - range_cast<3>(range<Dims>(zeros))};
const auto unnamed_error_message = fmt::format("Out-of-bounds access in kernel 'celerity::detail::acc_out_of_bounds_kernel<{}>' detected: Accessor 0 for buffer B0 attempted to "
"access indices between {} which are outside of mapped subrange {}", Dims, attempted_sr, subrange_cast<3>(accessible_sr));
CHECK_THAT(lc->get_log(), Catch::Matchers::ContainsSubstring(unnamed_error_message));

const auto named_error_message =
fmt::format("Out-of-bounds access in kernel 'celerity::detail::acc_out_of_bounds_kernel<{}>' detected: Accessor 1 for buffer {} attempted to "
"access indices between {} which are outside of mapped subrange {}",
Dims, buffer_name, attempted_sr, subrange_cast<3>(accessible_sr));
const auto named_error_message = fmt::format("Out-of-bounds access in kernel 'celerity::detail::acc_out_of_bounds_kernel<{}>' detected: Accessor 1 for buffer B1 \"{}\" attempted to "
"access indices between {} which are outside of mapped subrange {}", Dims, buffer_name, attempted_sr, subrange_cast<3>(accessible_sr));
CHECK_THAT(lc->get_log(), Catch::Matchers::ContainsSubstring(named_error_message));
}

Expand Down Expand Up @@ -738,16 +733,13 @@ namespace detail {
q.slow_full_sync();
}

const auto attempted_sr =
subrange<3>{id_cast<3>(oob_idx_lo), range_cast<3>(oob_idx_hi - oob_idx_lo + id_cast<Dims>(range<Dims>(ones))) - range_cast<3>(range<Dims>(zeros))};
const auto unnamed_error_message = fmt::format("Out-of-bounds access in host task detected: Accessor 0 for buffer 0 attempted to "
"access indices between {} which are outside of mapped subrange {}",
attempted_sr, subrange_cast<3>(accessible_sr));
const auto attempted_sr = subrange<3>{id_cast<3>(oob_idx_lo), range_cast<3>(oob_idx_hi - oob_idx_lo + id_cast<Dims>(range<Dims>(ones))) - range_cast<3>(range<Dims>(zeros))};
const auto unnamed_error_message = fmt::format("Out-of-bounds access in host task detected: Accessor 0 for buffer B0 attempted to "
"access indices between {} which are outside of mapped subrange {}", attempted_sr, subrange_cast<3>(accessible_sr));
CHECK_THAT(lc->get_log(), Catch::Matchers::ContainsSubstring(unnamed_error_message));

const auto named_error_message = fmt::format("Out-of-bounds access in host task detected: Accessor 1 for buffer {} attempted to "
"access indices between {} which are outside of mapped subrange {}",
buffer_name, attempted_sr, subrange_cast<3>(accessible_sr));
const auto named_error_message = fmt::format("Out-of-bounds access in host task detected: Accessor 1 for buffer B1 \"{}\" attempted to "
"access indices between {} which are outside of mapped subrange {}", buffer_name, attempted_sr, subrange_cast<3>(accessible_sr));
CHECK_THAT(lc->get_log(), Catch::Matchers::ContainsSubstring(named_error_message));
}
} // namespace detail
Expand Down
3 changes: 2 additions & 1 deletion test/debug_naming_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ TEST_CASE("debug names can be set and retrieved from tasks", "[debug]") {
TEST_CASE_METHOD(test_utils::runtime_fixture, "buffer_manager allows to set buffer debug names on buffers", "[buffer_manager]") {
celerity::buffer<int, 1> buff_a(16);
std::string buff_name{"my_buffer"};
std::string buffer_label{fmt::format("B{} \"{}\"", detail::get_buffer_id(buff_a), buff_name)};
celerity::detail::runtime::get_instance().get_buffer_manager().set_debug_name(celerity::detail::get_buffer_id(buff_a), buff_name);
CHECK(celerity::detail::runtime::get_instance().get_buffer_manager().get_debug_name(celerity::detail::get_buffer_id(buff_a)) == buff_name);
CHECK(detail::runtime::get_instance().get_buffer_manager().get_debug_label(detail::get_buffer_id(buff_a)) == buffer_label);
}


Expand Down

0 comments on commit eeebacb

Please sign in to comment.