Skip to content

Commit cae5b30

Browse files
committed
fixed globiter to be trivially copyable and added test, remove member myid from globiter
1 parent aec6e29 commit cae5b30

File tree

4 files changed

+55
-37
lines changed

4 files changed

+55
-37
lines changed

dash/include/dash/iterator/GlobIter.h

+12-36
Original file line numberDiff line numberDiff line change
@@ -134,80 +134,62 @@ class GlobIter
134134

135135
protected:
136136
/// Global memory used to dereference iterated values.
137-
GlobMemType * _globmem;
137+
GlobMemType * _globmem = nullptr;
138138
/// Pattern that specifies the iteration order (access pattern).
139-
const PatternType * _pattern;
139+
const PatternType * _pattern = nullptr;
140140
/// Current position of the iterator in global canonical index space.
141141
index_type _idx = 0;
142142
/// Maximum position allowed for this iterator.
143143
index_type _max_idx = 0;
144-
/// Unit id of the active unit
145-
team_unit_t _myid;
146144
/// Pointer to first element in local memory
147145
local_pointer _lbegin = nullptr;
148146

149147
public:
150-
/**
151-
* Default constructor.
152-
*/
153-
constexpr GlobIter()
154-
: _globmem(nullptr),
155-
_pattern(nullptr),
156-
_idx(0),
157-
_max_idx(0),
158-
_myid(dash::Team::All().myid()),
159-
_lbegin(nullptr)
160-
{ }
148+
149+
constexpr GlobIter() = default;
161150

162151
/**
163152
* Constructor, creates a global iterator on global memory following
164153
* the element order specified by the given pattern.
165154
*/
166155
constexpr GlobIter(
167156
GlobMemType * gmem,
168-
const PatternType & pat,
169-
index_type position = 0)
157+
const PatternType & pat,
158+
index_type position = 0)
170159
: _globmem(gmem),
171160
_pattern(&pat),
172161
_idx(position),
173162
_max_idx(pat.size() - 1),
174-
_myid(pat.team().myid()),
175163
_lbegin(_globmem->lbegin())
176164
{ }
177165

178166
/**
179167
* Copy constructor.
180168
*/
181169
template <
182-
class P_,
183-
class GM_,
184170
class Ptr_,
185171
class Ref_ >
186172
constexpr GlobIter(
187-
const GlobIter<nonconst_value_type, P_, GM_, Ptr_, Ref_> & other)
173+
const GlobIter<nonconst_value_type, PatternType, GlobMemType, Ptr_, Ref_> & other)
188174
: _globmem(other._globmem)
189175
, _pattern(other._pattern)
190176
, _idx (other._idx)
191177
, _max_idx(other._max_idx)
192-
, _myid (other._myid)
193178
, _lbegin (other._lbegin)
194179
{ }
195180

196181
/**
197182
* Move constructor.
198183
*/
199184
template <
200-
class P_,
201-
class GM_,
202185
class Ptr_,
203186
class Ref_ >
204187
constexpr GlobIter(
205-
GlobIter<nonconst_value_type, P_, GM_, Ptr_, Ref_> && other)
188+
GlobIter<nonconst_value_type, PatternType, GlobMemType, Ptr_, Ref_> && other)
206189
: _globmem(other._globmem)
207190
, _pattern(other._pattern)
208191
, _idx (other._idx)
209192
, _max_idx(other._max_idx)
210-
, _myid (other._myid)
211193
, _lbegin (other._lbegin)
212194
{ }
213195

@@ -216,18 +198,15 @@ class GlobIter
216198
*/
217199
template <
218200
typename T_,
219-
class P_,
220-
class GM_,
221201
class Ptr_,
222202
class Ref_ >
223203
self_t & operator=(
224-
const GlobIter<T_, P_, GM_, Ptr_, Ref_ > & other)
204+
const GlobIter<T_, PatternType, GlobMemType, Ptr_, Ref_ > & other)
225205
{
226206
_globmem = other._globmem;
227207
_pattern = other._pattern;
228208
_idx = other._idx;
229209
_max_idx = other._max_idx;
230-
_myid = other._myid;
231210
_lbegin = other._lbegin;
232211
return *this;
233212
}
@@ -237,18 +216,15 @@ class GlobIter
237216
*/
238217
template <
239218
typename T_,
240-
class P_,
241-
class GM_,
242219
class Ptr_,
243220
class Ref_ >
244221
self_t & operator=(
245-
GlobIter<T_, P_, GM_, Ptr_, Ref_ > && other)
222+
GlobIter<T_, PatternType, GlobMemType, Ptr_, Ref_ > && other)
246223
{
247224
_globmem = other._globmem;
248225
_pattern = other._pattern;
249226
_idx = other._idx;
250227
_max_idx = other._max_idx;
251-
_myid = other._myid;
252228
_lbegin = other._lbegin;
253229
// no ownership to transfer
254230
return *this;
@@ -430,7 +406,7 @@ class GlobIter
430406
*/
431407
constexpr bool is_local() const
432408
{
433-
return (_myid == lpos().unit);
409+
return (dash::Team::All().myid() == lpos().unit);
434410
}
435411

436412
/**
@@ -466,7 +442,7 @@ class GlobIter
466442
local_pos_t local_pos = _pattern->local(idx);
467443
DASH_LOG_TRACE_VAR("GlobIter.local= >", local_pos.unit);
468444
DASH_LOG_TRACE_VAR("GlobIter.local= >", local_pos.index);
469-
if (_myid != local_pos.unit) {
445+
if (dash::Team::All().myid() != local_pos.unit) {
470446
// Iterator position does not point to local element
471447
return nullptr;
472448
}

dash/test/types/GlobIterTest.cc

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
2+
#include "../TestBase.h"
3+
#include "../TestLogHelpers.h"
4+
#include "GlobIterTest.h"
5+
6+
#include <type_traits>
7+
#include <dash/Array.h>
8+
#include <dash/Atomic.h>
9+
10+
11+
TEST_F(GlobIterTest, IteratorTypes)
12+
{
13+
{
14+
using array_t = dash::Array<int>;
15+
static_assert(std::is_trivially_copyable<array_t::iterator>::value,
16+
"Array::iterator not trivially copyable");
17+
static_assert(std::is_trivially_copyable<array_t::const_iterator>::value,
18+
"Array::const_iterator not trivially copyable");
19+
}
20+
{
21+
using array_t = dash::Array<dash::Atomic<int>>;
22+
static_assert(std::is_trivially_copyable<array_t::iterator>::value,
23+
"Array<Atomic<T>>::iterator not trivially copyable");
24+
static_assert(std::is_trivially_copyable<array_t::const_iterator>::value,
25+
"Array<Atomic<T>>::const_iterator not trivially copyable");
26+
}
27+
}

dash/test/types/GlobIterTest.h

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#ifndef DASH__TEST__GLOBREF_TEST_H_
2+
#define DASH__TEST__GLOBREF_TEST_H_
3+
4+
#include <gtest/gtest.h>
5+
6+
#include "../TestBase.h"
7+
8+
9+
/**
10+
* Test fixture for global iterators
11+
*/
12+
class GlobIterTest: public dash::test::TestBase {
13+
};
14+
15+
#endif // DASH__TEST__GLOBREF_TEST_H_

0 commit comments

Comments
 (0)