Add new as<> wrapper around static_cast for use with isa<>

Mainly to silence clang-tidy warnings. Move isa<> to utils header.

Author: psalz

include/command.h

@@ -19,12 +19,6 @@ namespace detail {
 	// ------------------------------------------------ COMMAND GRAPH -------------------------------------------------
 	// ----------------------------------------------------------------------------------------------------------------
 
-	// TODO: Consider using LLVM-style RTTI for better performance
-	template <typename T, typename P>
-	bool isa(P* p) {
-		return dynamic_cast<T*>(const_cast<std::remove_const_t<P>*>(p)) != nullptr;
-	}
-
 	// TODO: Consider adding a mechanism (during debug builds?) to assert that dependencies can only exist between commands on the same node
 	class abstract_command : public intrusive_graph_node<abstract_command> {
 		friend class command_graph;

include/command_graph.h

@@ -140,13 +140,13 @@ namespace detail {
 			m_execution_front.erase(dependee);
 
 			// Sanity check: For non-dataflow dependencies the commands can only be of specific types
-			if(origin == dependency_origin::execution_front) { assert(isa<epoch_command>(depender) || isa<horizon_command>(depender)); }
+			if(origin == dependency_origin::execution_front) { assert(utils::isa<epoch_command>(depender) || utils::isa<horizon_command>(depender)); }
 			if(origin == dependency_origin::collective_group_serialization) {
-				assert(isa<execution_command>(depender));
+				assert(utils::isa<execution_command>(depender));
 				// The original execution command may have been subsumed by a horizon / epoch
-				assert(isa<execution_command>(dependee) || isa<epoch_command>(dependee) || isa<horizon_command>(dependee));
+				assert(utils::isa<execution_command>(dependee) || utils::isa<epoch_command>(dependee) || utils::isa<horizon_command>(dependee));
 			}
-			if(origin == dependency_origin::last_epoch) { assert(isa<epoch_command>(dependee) || isa<horizon_command>(dependee)); }
+			if(origin == dependency_origin::last_epoch) { assert(utils::isa<epoch_command>(dependee) || utils::isa<horizon_command>(dependee)); }
 
 			// Sanity check for unit tests, where we may have multiple CDAGS
 			assert(m_commands.at(depender->get_cid()).get() == depender);

include/utils.h

@@ -7,6 +7,17 @@
 
 namespace celerity::detail::utils {
 
+template <typename T, typename P>
+bool isa(const P* p) {
+	return dynamic_cast<const T*>(p) != nullptr;
+}
+
+template <typename T, typename P>
+auto as(P* p) {
+	assert(isa<T>(p));
+	return static_cast<std::conditional_t<std::is_const_v<P>, const T*, T*>>(p);
+}
+
 template <typename BitMaskT>
 constexpr inline uint32_t popcount(const BitMaskT bit_mask) noexcept {
 	static_assert(std::is_integral_v<BitMaskT> && std::is_unsigned_v<BitMaskT>, "popcount argument needs to be an unsigned integer type.");

src/distributed_graph_generator.cc

@@ -220,7 +220,7 @@ void distributed_graph_generator::generate_distributed_commands(const task& tsk)
 				// we have to include it in exactly one of the per-node intermediate reductions.
 				for(const auto& reduction : tsk.get_reductions()) {
 					if(nid == reduction_initializer_nid && reduction.init_from_buffer) {
-						static_cast<execution_command*>(cmd)->set_is_reduction_initializer(true);
+						utils::as<execution_command>(cmd)->set_is_reduction_initializer(true);
 						break;
 					}
 				}
@@ -313,7 +313,7 @@ void distributed_graph_generator::generate_distributed_commands(const task& tsk)
 								// possibly even multiple for partially already-replicated data.
 								// TODO: Can and/or should we consolidate?
 								auto* const push_cmd = create_command<push_command>(bid, 0, nid, trid, grid_box_to_subrange(replicated_box));
-								assert(!isa<await_push_command>(m_cdag.get(wcs)) && "Attempting to push non-owned data?!");
+								assert(!utils::isa<await_push_command>(m_cdag.get(wcs)) && "Attempting to push non-owned data?!");
 								m_cdag.add_dependency(push_cmd, m_cdag.get(wcs), dependency_kind::true_dep, dependency_origin::dataflow);
 								generated_pushes.push_back(push_cmd);
 
@@ -450,13 +450,13 @@ void distributed_graph_generator::generate_distributed_commands(const task& tsk)
 			assert(writer_cmd != nullptr);
 
 			// We're only interested in writes that happen within the same task as the push
-			if(isa<task_command>(writer_cmd) && static_cast<task_command*>(writer_cmd)->get_tid() == tsk.get_id()) {
+			if(utils::isa<task_command>(writer_cmd) && utils::as<task_command>(writer_cmd)->get_tid() == tsk.get_id()) {
 				// In certain situations the push might have a true dependency on the last writer,
 				// in that case don't add an anti-dependency (as that would cause a cycle).
 				// TODO: Is this still possible? We don't have a unit test exercising this branch...
 				if(push_cmd->has_dependency(writer_cmd, dependency_kind::true_dep)) {
 					// This can currently only happen for await_push commands.
-					assert(isa<await_push_command>(writer_cmd));
+					assert(utils::isa<await_push_command>(writer_cmd));
 					continue;
 				}
 				m_cdag.add_dependency(writer_cmd, push_cmd, dependency_kind::anti_dep, dependency_origin::dataflow);
@@ -495,7 +495,7 @@ void distributed_graph_generator::generate_anti_dependencies(
 	const auto last_writers = last_writers_map.get_region_values(write_req);
 	for(const auto& [box, wcs] : last_writers) {
 		auto* const last_writer_cmd = m_cdag.get(static_cast<command_id>(wcs));
-		assert(!isa<task_command>(last_writer_cmd) || static_cast<task_command*>(last_writer_cmd)->get_tid() != tid);
+		assert(!utils::isa<task_command>(last_writer_cmd) || utils::as<task_command>(last_writer_cmd)->get_tid() != tid);
 
 		// Add anti-dependencies onto all successors of the writer
 		bool has_successors = false;
@@ -506,7 +506,7 @@ void distributed_graph_generator::generate_anti_dependencies(
 			auto* const cmd = d.node;
 
 			// We might have already generated new commands within the same task that also depend on this; in that case, skip it
-			if(isa<task_command>(cmd) && static_cast<task_command*>(cmd)->get_tid() == tid) continue;
+			if(utils::isa<task_command>(cmd) && utils::as<task_command>(cmd)->get_tid() == tid) continue;
 
 			// So far we don't know whether the dependent actually intersects with the subrange we're writing
 			if(const auto command_reads_it = m_command_buffer_reads.find(cmd->get_cid()); command_reads_it != m_command_buffer_reads.end()) {
@@ -549,7 +549,7 @@ void distributed_graph_generator::process_task_side_effect_requirements(const ta
 
 void distributed_graph_generator::set_epoch_for_new_commands(const abstract_command* const epoch_or_horizon) {
 	// both an explicit epoch command and an applied horizon can be effective epochs
-	assert(isa<epoch_command>(epoch_or_horizon) || isa<horizon_command>(epoch_or_horizon));
+	assert(utils::isa<epoch_command>(epoch_or_horizon) || utils::isa<horizon_command>(epoch_or_horizon));
 
 	for(auto& [bid, bs] : m_buffer_states) {
 		bs.local_last_writer.apply_to_values([epoch_or_horizon](const write_command_state& wcs) {

src/executor.cc

@@ -86,7 +86,7 @@ namespace detail {
 					if(m_jobs[d].unsatisfied_dependencies == 0) { ready_jobs.push_back(d); }
 				}
 
-				if(isa<device_execute_job>(job_handle.job.get())) {
+				if(utils::isa<device_execute_job>(job_handle.job.get())) {
 					m_running_device_compute_jobs--;
 				} else if(const auto epoch = dynamic_cast<epoch_job*>(job_handle.job.get()); epoch && epoch->get_epoch_action() == epoch_action::shutdown) {
 					assert(m_command_queue.empty());
@@ -106,7 +106,7 @@ namespace detail {
 					auto* job = m_jobs.at(cid).job.get();
 					job->start();
 					job->update();
-					if(isa<device_execute_job>(job)) { m_running_device_compute_jobs++; }
+					if(utils::isa<device_execute_job>(job)) { m_running_device_compute_jobs++; }
 				}
 			}
 

src/graph_serializer.cc

@@ -27,9 +27,9 @@ namespace detail {
 		task_cmds.reserve(cmds.size() / 2); // Somewhat overzealous, we are likely to have more push commands
 
 		for(const auto& cmd : cmds) {
-			if(isa<push_command>(cmd)) {
+			if(utils::isa<push_command>(cmd)) {
 				push_cmds.push_back(cmd);
-			} else if(isa<task_command>(cmd)) {
+			} else if(utils::isa<task_command>(cmd)) {
 				task_cmds.push_back(cmd);
 			}
 		}
@@ -40,10 +40,10 @@ namespace detail {
 		const auto flush_recursive = [this, &check_tid, &flush_count](abstract_command* cmd, auto recurse) -> void {
 			(void)check_tid;
 #if defined(CELERITY_DETAIL_ENABLE_DEBUG)
-			if(isa<task_command>(cmd)) {
+			if(utils::isa<task_command>(cmd)) {
 				// Verify that all commands belong to the same task
-				assert(check_tid == task_id(-1) || check_tid == static_cast<task_command*>(cmd)->get_tid());
-				check_tid = static_cast<task_command*>(cmd)->get_tid();
+				assert(check_tid == task_id(-1) || check_tid == utils::as<task_command>(cmd)->get_tid());
+				check_tid = utils::as<task_command>(cmd)->get_tid();
 			}
 #endif
 			std::vector<command_id> deps;

src/print_graph.cc

@@ -170,7 +170,7 @@ namespace detail {
 			const auto id = local_to_global_id(cmd.get_cid());
 			const auto label = get_command_label(local_nid, cmd, tm, bm);
 			const auto* const fontcolor = colors[local_nid % (sizeof(colors) / sizeof(char*))];
-			const auto* const shape = isa<task_command>(&cmd) ? "box" : "ellipse";
+			const auto* const shape = utils::isa<task_command>(&cmd) ? "box" : "ellipse";
 			return fmt::format("{}[label=<{}> fontcolor={} shape={}];", id, label, fontcolor, shape);
 		};
 

test/distributed_graph_generator_test_utils.h

@@ -157,8 +157,8 @@ class command_query {
 			if(node_filter.has_value() && *node_filter != nid) continue;
 			for(const auto* cmd : m_commands_by_node[nid]) {
 				if(task_filter.has_value()) {
-					if(!isa<task_command>(cmd)) continue;
-					if(static_cast<const task_command*>(cmd)->get_tid() != *task_filter) continue;
+					if(!utils::isa<task_command>(cmd)) continue;
+					if(utils::as<task_command>(cmd)->get_tid() != *task_filter) continue;
 				}
 				if(type_filter.has_value()) {
 					if(get_type(cmd) != *type_filter) continue;
@@ -345,13 +345,13 @@ class command_query {
 	}
 
 	static command_type get_type(const abstract_command* cmd) {
-		if(isa<epoch_command>(cmd)) return command_type::epoch;
-		if(isa<horizon_command>(cmd)) return command_type::horizon;
-		if(isa<execution_command>(cmd)) return command_type::execution;
-		if(isa<push_command>(cmd)) return command_type::push;
-		if(isa<await_push_command>(cmd)) return command_type::await_push;
-		if(isa<reduction_command>(cmd)) return command_type::reduction;
-		if(isa<fence_command>(cmd)) return command_type::fence;
+		if(utils::isa<epoch_command>(cmd)) return command_type::epoch;
+		if(utils::isa<horizon_command>(cmd)) return command_type::horizon;
+		if(utils::isa<execution_command>(cmd)) return command_type::execution;
+		if(utils::isa<push_command>(cmd)) return command_type::push;
+		if(utils::isa<await_push_command>(cmd)) return command_type::await_push;
+		if(utils::isa<reduction_command>(cmd)) return command_type::reduction;
+		if(utils::isa<fence_command>(cmd)) return command_type::fence;
 		throw query_exception("Unknown command type");
 	}
 

test/graph_generation_tests.cc

@@ -61,13 +61,13 @@ TEST_CASE("command_graph keeps track of execution front", "[command_graph][comma
 TEST_CASE("isa<> RTTI helper correctly handles command hierarchies", "[rtti][command-graph]") {
 	command_graph cdag;
 	auto* const np = cdag.create<epoch_command>(task_manager::initial_epoch_task, epoch_action::none);
-	REQUIRE(isa<abstract_command>(np));
+	REQUIRE(utils::isa<abstract_command>(np));
 	auto* const hec = cdag.create<execution_command>(0, subrange<3>{});
-	REQUIRE(isa<execution_command>(hec));
+	REQUIRE(utils::isa<execution_command>(hec));
 	auto* const pc = cdag.create<push_command>(0, 0, 0, 0, subrange<3>{});
-	REQUIRE(isa<abstract_command>(pc));
+	REQUIRE(utils::isa<abstract_command>(pc));
 	auto* const apc = cdag.create<await_push_command>(0, 0, 0, GridRegion<3>{});
-	REQUIRE(isa<abstract_command>(apc));
+	REQUIRE(utils::isa<abstract_command>(apc));
 }
 
 TEST_CASE("distributed_graph_generator generates dependencies for execution commands", "[distributed_graph_generator][command-graph]") {