From ca328a219a15a024b0729c2613b77904331a26ce Mon Sep 17 00:00:00 2001 From: Sam Yates <yates@cscs.ch> Date: Fri, 31 Mar 2017 15:21:01 +0200 Subject: [PATCH] Add more general indirect access view. (#216) * Implement `indirect_view` for indexed access via `transform_view`. * Extend `transform_iterator` to permit non-const access to reference-returning functor results. * Replace use of `indexed_view` with `indirect_view`. * Fix missing cpu target for vectorized modcc outputs. --- mechanisms/CMakeLists.txt | 2 + modcc/cprinter.cpp | 4 +- modcc/cudaprinter.cpp | 3 +- modcc/simd_printer.hpp | 6 +-- src/backends/stimulus_gpu.hpp | 1 - src/backends/stimulus_multicore.hpp | 4 +- src/indexed_view.hpp | 39 ----------------- src/ion.hpp | 4 +- src/mechanism.hpp | 4 +- src/util/indirect.hpp | 44 +++++++++++++++++++ src/util/transform.hpp | 32 +++++++++++--- tests/unit/test_transform.cpp | 68 +++++++++++++++++++++++++++++ 12 files changed, 147 insertions(+), 64 deletions(-) delete mode 100644 src/indexed_view.hpp create mode 100644 src/util/indirect.hpp diff --git a/mechanisms/CMakeLists.txt b/mechanisms/CMakeLists.txt index 01e08e46..8521261e 100644 --- a/mechanisms/CMakeLists.txt +++ b/mechanisms/CMakeLists.txt @@ -13,8 +13,10 @@ if(NMC_VECTORIZE_TARGET STREQUAL "KNL") set(modcc_target "avx512") elseif(NMC_VECTORIZE_TARGET STREQUAL "AVX") set(modcc_opt "-O") + set(modcc_target "cpu") elseif(NMC_VECTORIZE_TARGET STREQUAL "AVX2") set(modcc_opt "-O") + set(modcc_target "cpu") else() set(modcc_target "cpu") endif() diff --git a/modcc/cprinter.cpp b/modcc/cprinter.cpp index a30e8253..d48555b4 100644 --- a/modcc/cprinter.cpp +++ b/modcc/cprinter.cpp @@ -57,7 +57,6 @@ std::string CPrinter::emit_source() { text_.add_line("using view = typename base::view;"); text_.add_line("using iview = typename base::iview;"); text_.add_line("using const_iview = typename base::const_iview;"); - text_.add_line("using indexed_view_type= typename base::indexed_view_type;"); text_.add_line("using ion_type = typename base::ion_type;"); text_.add_line(); @@ -589,8 +588,7 @@ void CPrinter::visit(APIMethod *e) { auto const& name = var->name(); auto const& index_name = var->external_variable()->index_name(); text_.add_gutter(); - if(var->is_read()) text_ << "const "; - text_ << "indexed_view_type " + index_name; + text_ << "auto " + index_name + " = util::indirect_view"; auto channel = var->external_variable()->ion_channel(); if(channel==ionKind::none) { text_ << "(" + index_name + "_, node_index_);\n"; diff --git a/modcc/cudaprinter.cpp b/modcc/cudaprinter.cpp index 904bf7b3..2885a1c5 100644 --- a/modcc/cudaprinter.cpp +++ b/modcc/cudaprinter.cpp @@ -136,7 +136,7 @@ CUDAPrinter::CUDAPrinter(Module &m, bool o) text_.add_line("template<typename Backend>"); text_.add_line("class " + class_name + " : public mechanism<Backend> {"); - text_.add_line("public: "); + text_.add_line("public:"); text_.increase_indentation(); text_.add_line("using base = mechanism<Backend>;"); text_.add_line("using typename base::value_type;"); @@ -148,7 +148,6 @@ CUDAPrinter::CUDAPrinter(Module &m, bool o) text_.add_line("using typename base::iview;"); text_.add_line("using typename base::const_iview;"); text_.add_line("using typename base::const_view;"); - text_.add_line("using typename base::indexed_view_type;"); text_.add_line("using typename base::ion_type;"); text_.add_line("using param_pack_type = " + module_name + "_ParamPack<value_type, size_type>;"); diff --git a/modcc/simd_printer.hpp b/modcc/simd_printer.hpp index 53b07222..4c9efe96 100644 --- a/modcc/simd_printer.hpp +++ b/modcc/simd_printer.hpp @@ -155,15 +155,13 @@ void SimdPrinter<Arch>::emit_indexed_view(LocalVariable* var, auto const& index_name = var->external_variable()->index_name(); text_.add_gutter(); - if (var->is_read()) - text_ << "const "; - if (decls.find(index_name) == decls.cend()) { - text_ << "indexed_view_type "; + text_ << "auto "; decls.insert(index_name); } text_ << index_name; + text_ << " = util::indirect_view"; auto channel = var->external_variable()->ion_channel(); if (channel == ionKind::none) { text_ << "(" + emit_member_name(index_name) + ", node_index_);\n"; diff --git a/src/backends/stimulus_gpu.hpp b/src/backends/stimulus_gpu.hpp index 3f3cfa70..3d2ac907 100644 --- a/src/backends/stimulus_gpu.hpp +++ b/src/backends/stimulus_gpu.hpp @@ -58,7 +58,6 @@ public: using view = typename base::view; using iview = typename base::iview; using const_iview = typename base::const_iview; - using indexed_view_type= typename base::indexed_view_type; using ion_type = typename base::ion_type; stimulus(view vec_v, view vec_i, iarray&& node_index): diff --git a/src/backends/stimulus_multicore.hpp b/src/backends/stimulus_multicore.hpp index 59deb6e1..4ca4f0ef 100644 --- a/src/backends/stimulus_multicore.hpp +++ b/src/backends/stimulus_multicore.hpp @@ -5,6 +5,7 @@ #include <mechanism.hpp> #include <algorithms.hpp> +#include <util/indirect.hpp> #include <util/pprintf.hpp> namespace nest{ @@ -24,7 +25,6 @@ public: using view = typename base::view; using iview = typename base::iview; using const_iview = typename base::const_iview; - using indexed_view_type= typename base::indexed_view_type; using ion_type = typename base::ion_type; stimulus(view vec_v, view vec_i, iarray&& node_index): @@ -80,7 +80,7 @@ public: if (amplitude.size() != size()) { throw std::domain_error("stimulus called with mismatched parameter size\n"); } - indexed_view_type vec_i(vec_i_, node_index_); + auto vec_i = util::indirect_view(vec_i_, node_index_); int n = size(); for(int i=0; i<n; ++i) { if (t>=delay[i] && t<(delay[i]+duration[i])) { diff --git a/src/indexed_view.hpp b/src/indexed_view.hpp deleted file mode 100644 index 55c0e0a3..00000000 --- a/src/indexed_view.hpp +++ /dev/null @@ -1,39 +0,0 @@ -#pragma once - -#include <memory/memory.hpp> - -namespace nest { -namespace mc { - -template <typename Backend> -struct indexed_view { - using backend = Backend; - using value_type = typename backend::value_type; - using size_type = typename backend::size_type; - using view = typename backend::view; - using const_iview = typename backend::const_iview; - using reference = typename view::reference; - using const_reference = typename view::const_reference; - - view data; - const_iview index; - - indexed_view(view v, const_iview i): - data(v), index(i) - {} - - std::size_t size() const { - return index.size(); - } - - reference operator[] (std::size_t i) { - return data[index[i]]; - } - - const_reference operator[] (std::size_t i) const { - return data[index[i]]; - } -}; - -} // namespace mc -} // namespace nest diff --git a/src/ion.hpp b/src/ion.hpp index 8fbe57f8..65ebe3c2 100644 --- a/src/ion.hpp +++ b/src/ion.hpp @@ -2,7 +2,7 @@ #include <array> #include <memory/memory.hpp> -#include <indexed_view.hpp> +#include <util/indirect.hpp> namespace nest { namespace mc { @@ -57,8 +57,6 @@ public : using view = typename backend::view; using const_iview = typename backend::const_iview; - using indexed_view_type = indexed_view<backend>; - ion() = default; ion(const std::vector<size_type>& idx) : diff --git a/src/mechanism.hpp b/src/mechanism.hpp index 922d73c3..8cde22d2 100644 --- a/src/mechanism.hpp +++ b/src/mechanism.hpp @@ -5,9 +5,9 @@ #include <string> #include <util/meta.hpp> -#include <indexed_view.hpp> #include <ion.hpp> #include <parameter_list.hpp> +#include <util/indirect.hpp> #include <util/make_unique.hpp> namespace nest { @@ -37,8 +37,6 @@ public: using const_view = typename backend::const_view; using const_iview = typename backend::const_iview; - using indexed_view_type = indexed_view<backend>; - using ion_type = ion<backend>; mechanism(view vec_v, view vec_i, iarray&& node_index): diff --git a/src/util/indirect.hpp b/src/util/indirect.hpp new file mode 100644 index 00000000..5a9bb5b4 --- /dev/null +++ b/src/util/indirect.hpp @@ -0,0 +1,44 @@ +#pragma once + +/* + * Given an underlying sequence S and an index map I, + * present an indirect view V of S such that + * V[i] = S[I[i]] for all valid indices i. + * + * Implemented as a transform view of the index map. + */ + +#include <util/deduce_return.hpp> +#include <util/transform.hpp> +#include <util/meta.hpp> + +namespace nest { +namespace mc { +namespace util { + +// Seq: random access sequence + +namespace impl { + template <typename Data> + struct indirect_accessor { + using reference = typename util::sequence_traits<Data>::reference; + + Data& data; + indirect_accessor(Data& data): data(data) {} + + template <typename I> + reference operator()(const I& i) const { return data[i]; } + }; +} + +template <typename RASeq, typename Seq> +auto indirect_view(RASeq& data, const Seq& index_map) +DEDUCED_RETURN_TYPE(transform_view(index_map, impl::indirect_accessor<RASeq>(data))); + +template <typename RASeq, typename Seq> +auto indirect_view(const RASeq& data, const Seq& index_map) +DEDUCED_RETURN_TYPE(transform_view(index_map, impl::indirect_accessor<const RASeq>(data))); + +} // namespace util +} // namespace mc +} // namespace nest diff --git a/src/util/transform.hpp b/src/util/transform.hpp index dfb654fa..da9165cf 100644 --- a/src/util/transform.hpp +++ b/src/util/transform.hpp @@ -36,12 +36,16 @@ class transform_iterator: public iterator_adaptor<transform_iterator<I, F>, I> { I& inner() { return inner_; } using inner_value_type = util::decay_t<decltype(*inner_)>; + using raw_value_type = typename std::result_of<F (inner_value_type)>::type; + + static constexpr bool present_lvalue = std::is_reference<raw_value_type>::value; + public: using typename base::difference_type; - using value_type = util::decay_t<typename std::result_of<F (inner_value_type)>::type>; - using pointer = const value_type*; - using reference = const value_type&; + using value_type = util::decay_t<raw_value_type>; + using pointer = typename std::conditional<present_lvalue, value_type*, const value_type*>::type; + using reference = typename std::conditional<present_lvalue, raw_value_type, const value_type&>::type; transform_iterator() = default; @@ -77,15 +81,18 @@ public: // forward and input iterator requirements - value_type operator*() const { + typename std::conditional<present_lvalue, reference, value_type>::type + operator*() const { return f_.cref()(*inner_); } - util::pointer_proxy<value_type> operator->() const { - return **this; + typename std::conditional<present_lvalue, pointer, util::pointer_proxy<value_type>>::type + operator->() const { + return pointer_impl(std::integral_constant<bool, present_lvalue>{}); } - value_type operator[](difference_type n) const { + typename std::conditional<present_lvalue, reference, value_type>::type + operator[](difference_type n) const { return *(*this+n); } @@ -101,6 +108,17 @@ public: template <typename Sentinel> bool operator!=(const Sentinel& s) const { return !(inner_==s); } + +private: + // helper routines for operator->(): need different implementations for + // lvalue and non-lvalue access. + util::pointer_proxy<value_type> pointer_impl(std::false_type) const { + return **this; + } + + pointer pointer_impl(std::true_type) const { + return &(**this); + } }; template <typename I, typename F> diff --git a/tests/unit/test_transform.cpp b/tests/unit/test_transform.cpp index e6f49a79..33d1522e 100644 --- a/tests/unit/test_transform.cpp +++ b/tests/unit/test_transform.cpp @@ -5,6 +5,8 @@ #include <vector> #include <util/range.hpp> +#include <util/indirect.hpp> +#include <util/span.hpp> #include <util/transform.hpp> using namespace nest::mc; @@ -54,3 +56,69 @@ TEST(transform, transform_view_sentinel) { EXPECT_EQ("HELLO", out); } +TEST(transform, pointer_access) { + struct item { + int x; + }; + item data[3]; + + auto r = util::transform_view(util::make_span(0u, 3u), [&](unsigned i) -> item& { return data[i]; }); + + int c=10; + for (auto i=r.begin(); i!=r.end(); ++i) { + i->x = c++; + } + + EXPECT_EQ(10, data[0].x); + EXPECT_EQ(11, data[1].x); + EXPECT_EQ(12, data[2].x); +} + +TEST(transform, pointer_proxy) { + struct item { + int x; + }; + auto r = util::transform_view(util::make_span(0, 3), [&](int i) { return item{13+i}; }); + + int c=13; + for (auto i=r.begin(); i!=r.end(); ++i) { + EXPECT_EQ(c++, i->x); + } +} + +TEST(indirect, fwd_index) { + std::istringstream string_indices("5 2 3 0 1 1 4"); + const double data[6] = {10., 11., 12., 13., 14., 15.}; + + auto indices = util::make_range(std::istream_iterator<int>(string_indices), std::istream_iterator<int>()); + auto permuted = util::indirect_view(data, indices); + + std::vector<double> result(permuted.begin(), permuted.end()); + std::vector<double> expected = {15., 12., 13., 10., 11., 11., 14.}; + + EXPECT_EQ(expected, result); +} + +TEST(indirect, modifying) { + unsigned map1[] = {0, 2, 4, 1, 3, 0}; + unsigned map2[] = {0, 1, 1, 1, 2}; + + std::vector<double> data = {-1, -1, -1}; + + auto permuted = util::indirect_view(util::indirect_view(data, map2), map1); + + // expected mapping: + // permuted[0] = data[0] + // permuted[1] = data[1] + // permuted[2] = data[2] + // permuted[3] = data[1] + // permuted[4] = data[1] + // permuted[5] = data[0] + + for (unsigned i = 0; i<util::size(permuted); ++i) { + permuted[i] = 10.+i; + } + std::vector<double> expected = {15., 14., 12.}; + + EXPECT_EQ(expected, data); +} -- GitLab