From 9cc68dc5c9a606746cab01ddd565498cc1f5284c Mon Sep 17 00:00:00 2001 From: Matthias Kretz Date: Fri, 21 Jul 2023 10:42:24 +0200 Subject: [PATCH 1/4] CMake: Improve build type defaults * Since we have -Og, -O0 (the default) is never a good idea. * Building the project without a build type (the cmake default) is "usually not desirable" (cmake docs), therefore default to Release with assertions enabled. * Document build types. * Warn if the user somehow managed to build without optimization. --- CMakeLists.txt | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 51c8f0ad2..e27c2732b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,6 +5,33 @@ set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_VISIBILITY_PRESET hidden) set(CMAKE_VISIBILITY_INLINES_HIDDEN 1) +if (CMAKE_CXX_COMPILER_ID MATCHES "(Clang|GNU|Intel)") + # -Og is a much more reasonable default for debugging. Also enable gdb extensions. + set(CMAKE_CXX_FLAGS_DEBUG "-Og -ggdb" CACHE INTERNAL + "Flags used by the compiler during debug builds.") + + # Add a build type that keeps runtime checks enabled + set(CMAKE_CXX_FLAGS_RELWITHASSERT "-O3" CACHE INTERNAL + "Flags used by the compiler during release builds containing runtime checks.") + + # The default value is often an empty string, but this is usually not desirable and one of the + # other standard build types is usually more appropriate. + if(NOT CMAKE_BUILD_TYPE) + set(CMAKE_BUILD_TYPE "RelWithAssert" CACHE STRING + "Choose the type of build. Options are: None Debug Release RelWithAssert RelWithDebInfo MinSizeRel.\n\ + - None: no compiler flags, defaults and target-specific flags apply\n\ + - Debug: best/complete debugging experience; as optimized as reasonable\n\ + - Release: full optimization; some runtime checks disabled\n\ + - RelWithAssert: full optimization; runtime checks enabled\n\ + - RelWithDebInfo: optimized; debug info; some runtime checks disabled" + FORCE) + endif(NOT CMAKE_BUILD_TYPE) + + if (CMAKE_BUILD_TYPE STREQUAL "" AND NOT CMAKE_CXX_FLAGS MATCHES "-O[123gs]") + message(WARNING "It seems you are compiling without optimization. Please set CMAKE_BUILD_TYPE or CMAKE_CXX_FLAGS.") + endif () +endif () + # Mainly for FMT set(CMAKE_POSITION_INDEPENDENT_CODE TRUE) From e112b2488869262d76b52e9ac7b2ec9fdb140e14 Mon Sep 17 00:00:00 2001 From: Matthias Kretz Date: Mon, 17 Jul 2023 11:57:54 +0200 Subject: [PATCH 2/4] Ensure that the node implementation matches expectations ... by testing for HasRequiredProcessFunction. If HasRequiredProcessFunction fails, be helpful about the cause of the problem via static_assert messages. math_bulk_op in bm_case1 implemented both process_one and process_bulk. Remove process_bulk to make it compile. This necessitates a move of the process_bulk_requires_ith_output_as_span test to a different node type. expect_sink in qa_scheduler implemented both process_one and process_bulk. Remove process_one. --- bench/bm_case1.cpp | 21 ------------------- include/node.hpp | 10 +++++++++ include/node_traits.hpp | 18 ++++++++++++++++ test/blocklib/core/unit-test/tag_monitors.hpp | 5 +++++ test/qa_scheduler.cpp | 5 ----- 5 files changed, 33 insertions(+), 26 deletions(-) diff --git a/bench/bm_case1.cpp b/bench/bm_case1.cpp index 33ce8f550..392a5f23f 100644 --- a/bench/bm_case1.cpp +++ b/bench/bm_case1.cpp @@ -81,22 +81,6 @@ class math_bulk_op : public fg::node, fg::IN, "unknown op"); } } - - [[nodiscard]] constexpr fg::work_return_status_t - process_bulk(std::span input, std::span output) const noexcept { - // classic for-loop - for (std::size_t i = 0; i < input.size(); i++) { - output[i] = process_one(input[i]); - } - - // C++17 algorithms - // std::transform(input.begin(), input.end(), output.begin(), [this](const T& elem) { return process_one(elem);}); - - // C++20 ranges - // std::ranges::transform(input, output.begin(), [this](const T& elem) { return process_one(elem); }); - - return fg::work_return_status_t::OK; - } }; template @@ -108,11 +92,6 @@ using add_bulk = math_bulk_op; template using sub_bulk = math_bulk_op; -// Clang 15 and 16 crash on the following static_assert -#ifndef __clang__ -static_assert(fg::traits::node::process_bulk_requires_ith_output_as_span, 0>); -#endif - // // This defines a new node type that has only type template parameters. // diff --git a/include/node.hpp b/include/node.hpp index 45561611b..0bf849a25 100644 --- a/include/node.hpp +++ b/include/node.hpp @@ -517,6 +517,16 @@ class node : protected std::tuple { */ work_return_t work_internal(std::size_t requested_work) noexcept { + if constexpr (not HasRequiredProcessFunction) { + if constexpr (HasProcessBulkFunction and HasProcessOneFunction) { + static_assert(HasRequiredProcessFunction, "Ambiguous node interface. The node type implements both `process_one` and `process_bulk`. Remove one of them."); + } else if constexpr (traits::node::can_process_bulk_by_value) { + static_assert(not traits::node::can_process_bulk_by_value, "Deduced function parameters of `process_bulk` must be passed *by reference not by value*."); + } else { + static_assert(HasRequiredProcessFunction, + "Missing or incorrect node interface. The node type must implement either `process_one` or `process_bulk` with arguments matching the port types."); + } + } using fair::graph::work_return_status_t; using input_types = traits::node::input_port_types; using output_types = traits::node::output_port_types; diff --git a/include/node_traits.hpp b/include/node_traits.hpp index 8b284b04d..aa42636be 100644 --- a/include/node_traits.hpp +++ b/include/node_traits.hpp @@ -228,6 +228,11 @@ struct dummy_input_span : public std::span { // NOSONAR constexpr void consume(std::size_t) noexcept; }; +template +struct dummy_copyable_input_span : public std::span { + constexpr void consume(std::size_t) noexcept; +}; + template struct dummy_output_span : public std::span { // NOSONAR dummy_output_span(const dummy_output_span &) = delete; // NOSONAR @@ -235,6 +240,11 @@ struct dummy_output_span : public std::span { // NOSONAR constexpr void publish(std::size_t) noexcept; }; +template +struct dummy_copyable_output_span : public std::span { + constexpr void publish(std::size_t) noexcept; +}; + template struct nothing_you_ever_wanted {}; @@ -256,6 +266,14 @@ concept can_process_bulk = requires(Node &n, typename meta::transform_types std::same_as; }; +template +concept can_process_bulk_by_value = requires(Node &n, typename meta::transform_types>::tuple_type inputs, + typename meta::transform_types>::tuple_type outputs) { + { + detail::can_process_bulk_invoke_test(n, inputs, outputs, std::make_index_sequence::size>(), std::make_index_sequence::size>()) + } -> std::same_as; +}; + /* * Satisfied if `Derived` has a member function `process_bulk` which can be invoked with a number of arguments matching the number of input and output ports. Input arguments must accept either a * std::span or any type satisfying ConsumableSpan. Output arguments must accept either a std::span or any type satisfying PublishableSpan, except for the I-th output argument, which diff --git a/test/blocklib/core/unit-test/tag_monitors.hpp b/test/blocklib/core/unit-test/tag_monitors.hpp index 4351e94ec..bb5d9690d 100644 --- a/test/blocklib/core/unit-test/tag_monitors.hpp +++ b/test/blocklib/core/unit-test/tag_monitors.hpp @@ -186,6 +186,11 @@ static_assert(not HasProcessOneFunction>); static_assert(HasRequiredProcessFunction>); +// Clang 15 and 16 crash on the following static_assert +#ifndef __clang__ +static_assert(traits::node::process_bulk_requires_ith_output_as_span, 0>); +#endif + static_assert(HasProcessOneFunction>); static_assert(not HasProcessOneFunction>); static_assert(not HasProcessBulkFunction>); diff --git a/test/qa_scheduler.cpp b/test/qa_scheduler.cpp index 65a64a285..d3f9e6520 100644 --- a/test/qa_scheduler.cpp +++ b/test/qa_scheduler.cpp @@ -76,11 +76,6 @@ class expect_sink : public fg::node, fg::INname()); - } }; template() * std::declval())> From a5f424a20ccfdaf11015872bd9ebdf3213c2dbde Mon Sep 17 00:00:00 2001 From: Matthias Kretz Date: Mon, 17 Jul 2023 11:58:53 +0200 Subject: [PATCH 3/4] Diagnose incorrect available_samples function signatures Declare available_samples as const in all node implementations. --- include/node.hpp | 4 ++++ test/blocklib/core/unit-test/tag_monitors.hpp | 2 +- test/qa_data_sink.cpp | 2 +- test/qa_scheduler.cpp | 2 +- test/qa_settings.cpp | 2 +- 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/include/node.hpp b/include/node.hpp index 0bf849a25..1a064960f 100644 --- a/include/node.hpp +++ b/include/node.hpp @@ -537,6 +537,10 @@ class node : protected std::tuple { std::size_t samples_to_process = 0; std::size_t n_samples_until_next_tag = std::numeric_limits::max(); // default: no tags in sight if constexpr (is_source_node) { + if constexpr (requires { &Derived::available_samples; }) { + static_assert( + requires(const Derived &d) { d.available_samples(d); }, "Incorrect signature for available_samples. Should be `(signed) size_t available_samples(const NodeType&) const`"); + } if constexpr (requires(const Derived &d) { { self().available_samples(d) } -> std::same_as>; }) { diff --git a/test/blocklib/core/unit-test/tag_monitors.hpp b/test/blocklib/core/unit-test/tag_monitors.hpp index bb5d9690d..0887dccbd 100644 --- a/test/blocklib/core/unit-test/tag_monitors.hpp +++ b/test/blocklib/core/unit-test/tag_monitors.hpp @@ -45,7 +45,7 @@ struct TagSource : public node> { std::int64_t n_samples_produced{ 0 }; constexpr std::make_signed_t - available_samples(const TagSource &) noexcept { + available_samples(const TagSource &) const noexcept { if constexpr (UseProcessOne == ProcessFunction::USE_PROCESS_ONE) { // '-1' -> DONE, produced enough samples return n_samples_max == n_samples_produced ? -1 : n_samples_max - n_samples_produced; diff --git a/test/qa_data_sink.cpp b/test/qa_data_sink.cpp index 78b14e47d..061c3f9a8 100644 --- a/test/qa_data_sink.cpp +++ b/test/qa_data_sink.cpp @@ -53,7 +53,7 @@ struct Source : public node> { } constexpr std::make_signed_t - available_samples(const Source &) noexcept { + available_samples(const Source &) const noexcept { // TODO unify with other test sources // split into chunks so that we have a single tag at index 0 (or none) auto ret = static_cast>(n_samples_max - n_samples_produced); diff --git a/test/qa_scheduler.cpp b/test/qa_scheduler.cpp index d3f9e6520..35cc477ef 100644 --- a/test/qa_scheduler.cpp +++ b/test/qa_scheduler.cpp @@ -42,7 +42,7 @@ class count_source : public fg::node, fg::OUTname = name_; } constexpr std::make_signed_t - available_samples(const count_source & /*d*/) noexcept { + available_samples(const count_source & /*d*/) const noexcept { const auto ret = static_cast>(N - _count); return ret > 0 ? ret : -1; // '-1' -> DONE, produced enough samples } diff --git a/test/qa_settings.cpp b/test/qa_settings.cpp index 11e674dc4..de06b805e 100644 --- a/test/qa_settings.cpp +++ b/test/qa_settings.cpp @@ -98,7 +98,7 @@ struct Source : public node> { } constexpr std::make_signed_t - available_samples(const Source & /*self*/) noexcept { + available_samples(const Source & /*self*/) const noexcept { const auto ret = static_cast>(n_samples_max - n_samples_produced); return ret > 0 ? ret : -1; // '-1' -> DONE, produced enough samples } From 9ecd828b97302899f4b0afe55cee2aedb22086a2 Mon Sep 17 00:00:00 2001 From: Matthias Kretz Date: Mon, 17 Jul 2023 11:59:24 +0200 Subject: [PATCH 4/4] [WIP] Decimator test --- test/CMakeLists.txt | 1 + test/qa_decimator.cpp | 96 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 test/qa_decimator.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 1b34313eb..9ece17cc6 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -29,6 +29,7 @@ endfunction() add_ut_test(qa_buffer) add_ut_test(qa_data_sink) +add_ut_test(qa_decimator) add_ut_test(qa_dynamic_node) add_ut_test(qa_dynamic_port) add_ut_test(qa_fft) diff --git a/test/qa_decimator.cpp b/test/qa_decimator.cpp new file mode 100644 index 000000000..51148ee7b --- /dev/null +++ b/test/qa_decimator.cpp @@ -0,0 +1,96 @@ +#include + +#include +#include +#include +#include +#include + +#include + +namespace fg = fair::graph; + +using std::size_t; +using ssize_t = std::make_signed_t; + +template +struct source : public fg::node, fg::OUT> { + ssize_t produced = 0; + T value = 1; + + constexpr ssize_t + available_samples(const source&) const noexcept { + return 64 - produced; + } + + constexpr fg::work_return_status_t + process_bulk(std::span out) noexcept { + if (available_samples(*this) <= 0) { + return fg::work_return_status_t::ERROR; + } + for (T &x : out) { + x = value++; + } + produced += ssize_t(out.size()); + const auto ret = (available_samples(*this) <= 0) ? fg::work_return_status_t::DONE : fg::work_return_status_t::OK; + fmt::println("source::process_bulk; produced = {}, returning {}", produced, int(ret)); + return ret; + } +}; + +template +struct drop_odd_samples : public fg::node, fg::IN, fg::OUT> { + bool even = true; + + constexpr fg::work_return_status_t + process_bulk(std::span in, fg::PublishableSpan auto& out) noexcept { + boost::ut::expect(in.size() <= 15); + boost::ut::expect(out.size() <= 15); + const size_t to_publish = (in.size() + even) / 2; + boost::ut::expect(to_publish <= out.size()); + for (size_t i = 0; i < in.size(); ++i) { + if (even) { + out[i / 2] = in[i]; + } + even = !even; + } + out.publish(to_publish); + fmt::println("drop_odd_samples::process_bulk received {} samples and published {} samples", in.size(), to_publish); + return fg::work_return_status_t::OK; + } +}; + +template +struct sink : public fg::node, fg::IN> { + T expect_next = 1; + + constexpr void + process_one(T x) noexcept { + using namespace boost::ut; + expect(eq(x, expect_next)); + expect_next += increment; + } +}; + +const boost::ut::suite simple_decimator = [] { + using namespace boost::ut; + + "drop odd"_test = [] { + fg::graph flow_graph; + + auto &n0 = flow_graph.make_node>(); + auto &n1 = flow_graph.make_node>(); + auto &n2 = flow_graph.make_node>(); + + expect(eq(flow_graph.connect<"out">(n0).to<"in">(n1), fg::connection_result_t::SUCCESS)); + expect(eq(flow_graph.connect<"out">(n1).to<"in">(n2), fg::connection_result_t::SUCCESS)); + + fg::scheduler::simple sched{ std::move(flow_graph) }; + fmt::println("drop odd starts"); + sched.start(); + fmt::println("drop odd done"); + }; +}; + +int +main() {}