From 7375c35c3b7026465f00fa4701912f6eae87aa29 Mon Sep 17 00:00:00 2001 From: Matthias Kretz Date: Thu, 5 Sep 2024 13:39:58 +0200 Subject: [PATCH] Remove all offsets from processOne(_simd) Remove: - invokeProcessOneWithOrWithoutOffset - exact_argument_type - can_processOne_with_offset_invoke_test - can_processOne_simd_with_offset - can_processOne_scalar_with_offset - can_processOne_with_offset Modify MergedGraph apply_left, apply_right, processOne, and processOne_simd to not require an offset argument anymore. Modify all calls to processOne(_simd) to never pass an offset. Modify TagMonitors processOne to generateTag without offset. The published tag is now unconditional 0 all the time. Signed-off-by: Matthias Kretz --- .../gnuradio-4.0/testing/NullSources.hpp | 2 +- .../gnuradio-4.0/testing/TagMonitors.hpp | 8 ++-- core/benchmarks/bm-nosonar_node_api.cpp | 4 +- core/include/gnuradio-4.0/Block.hpp | 35 +++++++---------- core/include/gnuradio-4.0/BlockTraits.hpp | 39 +++++-------------- core/include/gnuradio-4.0/Graph.hpp | 34 +++++++--------- core/src/main.cpp | 12 +++--- core/test/qa_Block.cpp | 4 +- 8 files changed, 52 insertions(+), 86 deletions(-) diff --git a/blocks/testing/include/gnuradio-4.0/testing/NullSources.hpp b/blocks/testing/include/gnuradio-4.0/testing/NullSources.hpp index f3475a68..e7215019 100644 --- a/blocks/testing/include/gnuradio-4.0/testing/NullSources.hpp +++ b/blocks/testing/include/gnuradio-4.0/testing/NullSources.hpp @@ -168,7 +168,7 @@ Commonly used for testing scenarios and signal termination where output is unnec template V> void processOne(V) noexcept { if constexpr (stdx::is_simd_v) { - count += V::size(); + count += gr::Size_t(V::size()); } else { count++; } diff --git a/blocks/testing/include/gnuradio-4.0/testing/TagMonitors.hpp b/blocks/testing/include/gnuradio-4.0/testing/TagMonitors.hpp index 93f3b277..51841736 100644 --- a/blocks/testing/include/gnuradio-4.0/testing/TagMonitors.hpp +++ b/blocks/testing/include/gnuradio-4.0/testing/TagMonitors.hpp @@ -119,10 +119,10 @@ struct TagSource : public Block> { } } - T processOne(std::size_t offset) noexcept + T processOne() noexcept requires(UseProcessVariant == ProcessFunction::USE_PROCESS_ONE) { - const auto [tagGenerated, tagRepeatStarted] = generateTag("processOne(...)", offset); + const auto [tagGenerated, tagRepeatStarted] = generateTag("processOne(...)"); _nSamplesProduced++; if (!isInfinite() && _nSamplesProduced >= n_samples_max) { this->requestStop(); @@ -192,7 +192,7 @@ struct TagSource : public Block> { } private: - [[nodiscard]] auto generateTag(std::string_view processFunctionName, std::size_t offset = 0) { + [[nodiscard]] auto generateTag(std::string_view processFunctionName) { struct { bool tagGenerated = false; bool tagRepeatStarted = false; @@ -203,7 +203,7 @@ struct TagSource : public Block> { if (verbose_console) { print_tag(_tags[_tagIndex], fmt::format("{}::{}\t publish tag at {:6}", this->name.value, processFunctionName, _nSamplesProduced)); } - out.publishTag(_tags[_tagIndex].map, static_cast(offset)); // indices > 0 write tags in the future ... handle with care + out.publishTag(_tags[_tagIndex].map, static_cast(0)); // indices > 0 write tags in the future ... handle with care this->_outputTagsChanged = true; _tagIndex++; if (repeat_tags && _tagIndex == _tags.size()) { diff --git a/core/benchmarks/bm-nosonar_node_api.cpp b/core/benchmarks/bm-nosonar_node_api.cpp index a2355b79..2027a3a7 100644 --- a/core/benchmarks/bm-nosonar_node_api.cpp +++ b/core/benchmarks/bm-nosonar_node_api.cpp @@ -332,12 +332,12 @@ void loop_over_processOne(auto& node) { bm::test::n_samples_consumed = 0LU; #if DISABLE_SIMD for (std::size_t i = 0; i < N_SAMPLES; i++) { - node.processOne(i); + node.processOne(); } #else constexpr int N = 32; for (std::size_t i = 0; i < N_SAMPLES / N; i++) { - node.template processOne_simd(i, std::integral_constant{}); + node.template processOne_simd(std::integral_constant{}); } #endif expect(eq(bm::test::n_samples_produced, N_SAMPLES)) << "produced too many/few samples"; diff --git a/core/include/gnuradio-4.0/Block.hpp b/core/include/gnuradio-4.0/Block.hpp index 71cd6453..3fc9db5f 100644 --- a/core/include/gnuradio-4.0/Block.hpp +++ b/core/include/gnuradio-4.0/Block.hpp @@ -46,15 +46,6 @@ constexpr auto simdize_tuple_load_and_apply(auto width, const std::tuple& return [&](std::index_sequence) { return fun(std::tuple_element_t(std::ranges::data(std::get(rngs)) + offset, f)...); }(std::make_index_sequence()); } -template -auto invokeProcessOneWithOrWithoutOffset(T& node, std::size_t offset, const Us&... inputs) { - if constexpr (traits::block::can_processOne_with_offset) { - return node.processOne(offset, inputs...); - } else { - return node.processOne(inputs...); - } -} - template [[nodiscard]] constexpr auto& inputPort(Self* self) noexcept { using TRequestedPortType = typename traits::block::ports_data::template for_type::input_ports::template at; @@ -756,30 +747,30 @@ class Block : public lifecycle::StateMachine, public std::tuple - constexpr auto invoke_processOne(std::size_t offset, Ts&&... inputs) { + constexpr auto invoke_processOne(Ts&&... inputs) { if constexpr (traits::block::stream_output_ports::size == 0) { - invokeProcessOneWithOrWithoutOffset(self(), offset, std::forward(inputs)...); + self().processOne(std::forward(inputs)...); return std::tuple{}; } else if constexpr (traits::block::stream_output_ports::size == 1) { - return std::tuple{invokeProcessOneWithOrWithoutOffset(self(), offset, std::forward(inputs)...)}; + return std::tuple{self().processOne(std::forward(inputs)...)}; } else { - return invokeProcessOneWithOrWithoutOffset(self(), offset, std::forward(inputs)...); + return self().processOne(std::forward(inputs)...); } } template - constexpr auto invoke_processOne_simd(std::size_t offset, auto width, Ts&&... input_simds) { + constexpr auto invoke_processOne_simd(auto width, Ts&&... input_simds) { if constexpr (sizeof...(Ts) == 0) { if constexpr (traits::block::stream_output_ports::size == 0) { - self().processOne_simd(offset, width); + self().processOne_simd(width); return std::tuple{}; } else if constexpr (traits::block::stream_output_ports::size == 1) { - return std::tuple{self().processOne_simd(offset, width)}; + return std::tuple{self().processOne_simd(width)}; } else { - return self().processOne_simd(offset, width); + return self().processOne_simd(width); } } else { - return invoke_processOne(offset, std::forward(input_simds)...); + return invoke_processOne(std::forward(input_simds)...); } } @@ -1342,12 +1333,12 @@ class Block : public lifecycle::StateMachine, public std::tuple, public std::tupleinvoke_processOne(i, inputs[i]...); }, inputSpans); + auto results = std::apply([this, i](auto&... inputs) { return this->invoke_processOne(inputs[i]...); }, inputSpans); meta::tuple_for_each([i](auto& output_range, R&& result) { output_range[i] = std::forward(result); }, outputSpans, results); } return work::Status::OK; @@ -1374,7 +1365,7 @@ class Block : public lifecycle::StateMachine, public std::tupleinvoke_processOne(i, inputs[i]...); }, inputSpans); + auto results = std::apply([this, i](auto&... inputs) { return this->invoke_processOne(inputs[i]...); }, inputSpans); meta::tuple_for_each( [i](auto& output_range, R&& result) { if constexpr (meta::array_or_vector_type>) { diff --git a/core/include/gnuradio-4.0/BlockTraits.hpp b/core/include/gnuradio-4.0/BlockTraits.hpp index 98f42d31..1339a323 100644 --- a/core/include/gnuradio-4.0/BlockTraits.hpp +++ b/core/include/gnuradio-4.0/BlockTraits.hpp @@ -206,19 +206,17 @@ template using get_port_member_descriptor = typename meta::to_typelist>::template filter::template filter::template matches>::template at<0>; // TODO: Why is this not done with requires? +// mkretz: I don't understand the question. "this" in the question is unclear. +/* Helper to determine the return type of `block.processOne` for the given inputs. + * + * This helper is necessary because we need a pack of indices to expand the input tuples. In princple we should be able + * to use std::apply to the same effect. Except that `block` would need to be the first element of the tuple. This here + * is simpler and cheaper. + */ namespace detail { template auto can_processOne_invoke_test(auto& block, const auto& input, std::index_sequence) -> decltype(block.processOne(std::get(input)...)); -template -struct exact_argument_type { - template U> - constexpr operator U() const noexcept; -}; - -template -auto can_processOne_with_offset_invoke_test(auto& block, const auto& input, std::index_sequence) -> decltype(block.processOne(exact_argument_type(), std::get(input)...)); - template using simd_return_type_of_can_processOne = vir::simdize, vir::simdize>::size()>; } // namespace detail @@ -230,7 +228,7 @@ using simd_return_type_of_can_processOne = vir::simdize concept can_processOne_simd = // @@ -246,17 +244,6 @@ concept can_processOne_simd_const = { detail::can_processOne_invoke_test(block, input_simds, std::make_index_sequence::size()>()) } -> std::same_as>; }; -template -concept can_processOne_simd_with_offset = -#if DISABLE_SIMD - false; -#else - traits::block::stream_input_ports::template all_of and // checks we don't have port collections inside - traits::block::stream_input_port_types::size() > 0 && requires(TBlock& block, const vir::simdize>& input_simds) { - { detail::can_processOne_with_offset_invoke_test(block, input_simds, std::make_index_sequence::size()>()) } -> std::same_as>; - }; -#endif - template concept can_processOne_scalar = requires(TBlock& block, const stream_input_port_types_tuple& inputs) { { detail::can_processOne_invoke_test(block, inputs, std::make_index_sequence::size()>()) } -> std::same_as>; @@ -268,19 +255,11 @@ concept can_processOne_scalar_const = requires(const TBlock& block, const stream }; template -concept can_processOne_scalar_with_offset = requires(TBlock& block, const stream_input_port_types_tuple& inputs) { - { detail::can_processOne_with_offset_invoke_test(block, inputs, std::make_index_sequence::size()>()) } -> std::same_as>; -}; - -template -concept can_processOne = can_processOne_scalar or can_processOne_simd or can_processOne_scalar_with_offset or can_processOne_simd_with_offset; +concept can_processOne = can_processOne_scalar or can_processOne_simd; template concept can_processOne_const = can_processOne_scalar_const or can_processOne_simd_const; -template -concept can_processOne_with_offset = can_processOne_scalar_with_offset or can_processOne_simd_with_offset; - template concept can_processMessagesForPortConsumableSpan = requires(TBlock& block, TPort& inPort) { block.processMessages(inPort, inPort.streamReader().get(1UZ)); }; diff --git a/core/include/gnuradio-4.0/Graph.hpp b/core/include/gnuradio-4.0/Graph.hpp index 5be22097..32f68846 100644 --- a/core/include/gnuradio-4.0/Graph.hpp +++ b/core/include/gnuradio-4.0/Graph.hpp @@ -722,17 +722,17 @@ class MergedGraph : public Block, meta::co } template - constexpr auto apply_left(std::size_t offset, auto&& input_tuple) noexcept { - return [&](std::index_sequence) { return invokeProcessOneWithOrWithoutOffset(left, offset, std::get(std::forward(input_tuple))...); }(std::make_index_sequence()); + constexpr auto apply_left(auto&& input_tuple) noexcept { + return [&](std::index_sequence) { return left.processOne(std::get(std::forward(input_tuple))...); }(std::make_index_sequence()); } template - constexpr auto apply_right(std::size_t offset, auto&& input_tuple, auto&& tmp) noexcept { + constexpr auto apply_right(auto&& input_tuple, auto&& tmp) noexcept { return [&](std::index_sequence, std::index_sequence) { constexpr std::size_t first_offset = traits::block::stream_input_port_types::size; constexpr std::size_t second_offset = traits::block::stream_input_port_types::size + sizeof...(Is); static_assert(second_offset + sizeof...(Js) == std::tuple_size_v>); - return invokeProcessOneWithOrWithoutOffset(right, offset, std::get(std::forward(input_tuple))..., std::forward(tmp), std::get(input_tuple)...); + return right.processOne(std::get(std::forward(input_tuple))..., std::forward(tmp), std::get(input_tuple)...); }(std::make_index_sequence(), std::make_index_sequence()); } @@ -754,48 +754,44 @@ class MergedGraph : public Block, meta::co template requires traits::block::can_processOne_simd and traits::block::can_processOne_simd - constexpr vir::simdize processOne(std::size_t offset, const Ts&... inputs) { + constexpr vir::simdize processOne(const Ts&... inputs) { static_assert(traits::block::stream_output_port_types::size == 1, "TODO: SIMD for multiple output ports not implemented yet"); - return apply_right::size() - InId - 1>(offset, std::tie(inputs...), apply_left::size()>(offset, std::tie(inputs...))); + return apply_right::size() - InId - 1>(std::tie(inputs...), apply_left::size()>(std::tie(inputs...))); } - constexpr auto processOne_simd(std::size_t offset, auto N) + constexpr auto processOne_simd(auto N) requires traits::block::can_processOne_simd { if constexpr (requires(Left& l) { - { l.processOne_simd(offset, N) }; + { l.processOne_simd(N) }; }) { - return invokeProcessOneWithOrWithoutOffset(right, offset, left.processOne_simd(offset, N)); - } else if constexpr (requires(Left& l) { - { l.processOne_simd(N) }; - }) { - return invokeProcessOneWithOrWithoutOffset(right, offset, left.processOne_simd(N)); + return right.processOne(left.processOne_simd(N)); } else { using LeftResult = typename traits::block::stream_return_type; using V = vir::simdize; alignas(stdx::memory_alignment_v) LeftResult tmp[V::size()]; for (std::size_t i = 0UZ; i < V::size(); ++i) { - tmp[i] = invokeProcessOneWithOrWithoutOffset(left, offset + i); + tmp[i] = left.processOne(); } - return invokeProcessOneWithOrWithoutOffset(right, offset, V(tmp, stdx::vector_aligned)); + return right.processOne(V(tmp, stdx::vector_aligned)); } } template // Nicer error messages for the following would be good, but not at the expense of breaking can_processOne_simd. requires(TInputPortTypes::template are_equal...>) - constexpr TReturnType processOne(std::size_t offset, Ts&&... inputs) { + constexpr TReturnType processOne(Ts&&... inputs) { // if (sizeof...(Ts) == 0) we could call `return processOne_simd(integral_constant)`. But if // the caller expects to process *one* sample (no inputs for the caller to explicitly // request simd), and we process more, we risk inconsistencies. if constexpr (traits::block::stream_output_port_types::size == 1) { // only the result from the right block needs to be returned - return apply_right::size() - InId - 1>(offset, std::forward_as_tuple(std::forward(inputs)...), apply_left::size()>(offset, std::forward_as_tuple(std::forward(inputs)...))); + return apply_right::size() - InId - 1>(std::forward_as_tuple(std::forward(inputs)...), apply_left::size()>(std::forward_as_tuple(std::forward(inputs)...))); } else { // left produces a tuple - auto left_out = apply_left::size()>(offset, std::forward_as_tuple(std::forward(inputs)...)); - auto right_out = apply_right::size() - InId - 1>(offset, std::forward_as_tuple(std::forward(inputs)...), std::move(std::get(left_out))); + auto left_out = apply_left::size()>(std::forward_as_tuple(std::forward(inputs)...)); + auto right_out = apply_right::size() - InId - 1>(std::forward_as_tuple(std::forward(inputs)...), std::move(std::get(left_out))); if constexpr (traits::block::stream_output_port_types::size == 2 && traits::block::stream_output_port_types::size == 1) { return std::make_tuple(std::move(std::get(left_out)), std::move(right_out)); diff --git a/core/src/main.cpp b/core/src/main.cpp index cbf932db..7a6a3970 100644 --- a/core/src/main.cpp +++ b/core/src/main.cpp @@ -104,7 +104,7 @@ int main() { int r = 0; for (std::size_t i = 0; i < 4; ++i) { - r += merged.processOne(i, a[i], b[i]); + r += merged.processOne(a[i], b[i]); } fmt::print("Result of graph execution: {}\n", r); @@ -119,7 +119,7 @@ int main() { std::array a = {1, 2, 3, 4}; for (std::size_t i = 0; i < 4; ++i) { - auto tuple = merged.processOne(i, a[i]); + auto tuple = merged.processOne(a[i]); auto [r1, r2] = tuple; fmt::print("{} {} \n", r1, r2); } @@ -134,7 +134,7 @@ int main() { int r = 0; for (std::size_t i = 0; i < 4; ++i) { - r += merged.processOne(i, a[i], b[i]); + r += merged.processOne(a[i], b[i]); } fmt::print("Result of graph execution: {}\n", r); @@ -149,7 +149,7 @@ int main() { std::array a = {1, 2, 3, 4}; for (std::size_t i = 0; i < 4; ++i) { - auto tuple = merged.processOne(i, a[i]); + auto tuple = merged.processOne(a[i]); auto [r1, r2, r3, r4] = tuple; fmt::print("{} {} {} {} \n", r1, r2, r3, r4); } @@ -161,13 +161,13 @@ int main() { auto random = CountSource{}; auto merged = mergeByIndex<0, 0>(std::move(random), ExpectSink()); - merged.processOne(0); + merged.processOne(); } { auto random = CountSource{}; auto merged = merge<"random", "original">(std::move(random), scale()); - merged.processOne(0); + merged.processOne(); } } diff --git a/core/test/qa_Block.cpp b/core/test/qa_Block.cpp index c2e0e180..41abfe8d 100644 --- a/core/test/qa_Block.cpp +++ b/core/test/qa_Block.cpp @@ -31,8 +31,8 @@ static_assert(traits::block::stream_input_port_types::size() == 1); static_assert(std::same_as, float>); static_assert(traits::block::can_processOne_scalar); static_assert(traits::block::can_processOne_simd); -static_assert(traits::block::can_processOne_scalar_with_offset(copy(), copy()))>); -static_assert(traits::block::can_processOne_simd_with_offset(copy(), copy()))>); +static_assert(traits::block::can_processOne_scalar(copy(), copy()))>); +static_assert(traits::block::can_processOne_simd(copy(), copy()))>); static_assert(SourceBlockLike); static_assert(SinkBlockLike); static_assert(SourceBlockLike(copy(), copy()))>);