Skip to content

Commit 6851145

Browse files
facuMHpsalz
authored andcommitted
Remove error message for buffer deallocation.
Add test for correct deallocation.
1 parent 2368b4c commit 6851145

File tree

2 files changed

+36
-10
lines changed

2 files changed

+36
-10
lines changed

src/runtime.cc

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -232,16 +232,7 @@ namespace detail {
232232
if(is_master_node()) ggen->add_buffer(bid, info.range);
233233
}
234234

235-
void runtime::handle_buffer_unregistered(buffer_id bid) {
236-
// If the runtime is still active, and at least one task (other than the init task) has been submitted, report an error.
237-
// TODO: This is overly restrictive. Can we instead check whether any tasks that require this particular buffer are still pending?
238-
if(is_active && task_mngr->get_total_task_count() > 1) {
239-
// We cannot throw here, as this is being called from buffer destructors.
240-
default_logger->error(
241-
"The Celerity runtime detected that a buffer is going out of scope before all tasks have been completed. This is not allowed.");
242-
}
243-
maybe_destroy_runtime();
244-
}
235+
void runtime::handle_buffer_unregistered(buffer_id bid) { maybe_destroy_runtime(); }
245236

246237
void runtime::maybe_destroy_runtime() const {
247238
if(is_active) return;

test/runtime_tests.cc

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,41 @@ namespace detail {
578578
REQUIRE_FALSE(bm.has_active_buffers());
579579
}
580580

581+
TEST_CASE("buffer_manager allows buffer deallocation", "[buffer_manager][dealloc]") {
582+
celerity::distr_queue q;
583+
buffer_id b_id;
584+
auto& bm = runtime::get_instance().get_buffer_manager();
585+
auto& tm = runtime::get_instance().get_task_manager();
586+
constexpr int new_horizon_step = 2;
587+
tm.set_horizon_step(new_horizon_step);
588+
{
589+
celerity::buffer<int, 1> b(celerity::range<1>(128));
590+
b_id = celerity::detail::get_buffer_id(b);
591+
q.submit([=](celerity::handler& cgh) {
592+
celerity::accessor a{b, cgh, celerity::access::all(), celerity::write_only};
593+
cgh.parallel_for<class UKN(i)>(b.get_range(), [=](celerity::item<1> it) {});
594+
});
595+
REQUIRE(bm.has_buffer(b_id));
596+
}
597+
celerity::buffer<int, 1> c(celerity::range<1>(128));
598+
// we need horizon_step_size * 3 + 1 tasks to generate the third horizon,
599+
// and one extra task to trigger the clean_up process
600+
for(int i = 0; i < (new_horizon_step * 3 + 2); i++) {
601+
q.submit([=](celerity::handler& cgh) {
602+
celerity::accessor a{c, cgh, celerity::access::all(), celerity::write_only};
603+
cgh.parallel_for<class UKN(i)>(c.get_range(), [=](celerity::item<1>) {});
604+
});
605+
// this sync is inside the loop because otherwise there is a race between the prepass and the executor informing the TDAG
606+
// of the executed horizons, meaning that task deletion is not guaranteed.
607+
q.slow_full_sync();
608+
}
609+
// require buffer b was indeed unregistered.
610+
REQUIRE_FALSE(bm.has_buffer(b_id));
611+
612+
// TODO: check whether error was printed or not
613+
maybe_print_graph(celerity::detail::runtime::get_instance().get_task_manager());
614+
}
615+
581616
TEST_CASE_METHOD(test_utils::buffer_manager_fixture, "buffer_manager creates appropriately sized buffers as needed", "[buffer_manager]") {
582617
auto& bm = get_buffer_manager();
583618
auto bid = bm.register_buffer<float, 1>(cl::sycl::range<3>(3072, 1, 1));

0 commit comments

Comments
 (0)