Skip to content

Commit c0929c3

Browse files
[NFC][SYCL] Simplify variadic_iterator usage
`std::weak_ptr<node_impl>` aren't used to create `nodes_range` anymore (#19438 being the last change that enabled that, I think) and we also added `SyclTy` template parameter to `variadic_iterator` so that the dereference operator can be implemented in a generic way in the base class.
1 parent 3d1002b commit c0929c3

File tree

3 files changed

+19
-50
lines changed

3 files changed

+19
-50
lines changed

sycl/source/detail/device_impl.hpp

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2282,24 +2282,10 @@ class device_impl : public std::enable_shared_from_this<device_impl> {
22822282

22832283
}; // class device_impl
22842284

2285-
struct devices_deref_impl {
2286-
template <typename T> static device_impl &dereference(T &Elem) {
2287-
using Ty = std::decay_t<decltype(Elem)>;
2288-
if constexpr (std::is_same_v<Ty, device>) {
2289-
return *getSyclObjImpl(Elem);
2290-
} else if constexpr (std::is_same_v<Ty, device_impl>) {
2291-
return Elem;
2292-
} else {
2293-
return *Elem;
2294-
}
2295-
}
2296-
};
2297-
using devices_iterator =
2298-
variadic_iterator<devices_deref_impl, device,
2299-
std::vector<std::shared_ptr<device_impl>>::const_iterator,
2300-
std::vector<device>::const_iterator,
2301-
std::vector<device_impl *>::const_iterator,
2302-
device_impl *>;
2285+
using devices_iterator = variadic_iterator<
2286+
device, std::vector<std::shared_ptr<device_impl>>::const_iterator,
2287+
std::vector<device>::const_iterator,
2288+
std::vector<device_impl *>::const_iterator, device_impl *>;
23032289

23042290
class devices_range : public iterator_range<devices_iterator> {
23052291
private:

sycl/source/detail/graph/node_impl.hpp

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -749,37 +749,13 @@ class node_impl : public std::enable_shared_from_this<node_impl> {
749749
}
750750
};
751751

752-
struct nodes_deref_impl {
753-
template <typename T> static node_impl &dereference(T &Elem) {
754-
using Ty = std::decay_t<decltype(Elem)>;
755-
if constexpr (std::is_same_v<Ty, std::weak_ptr<node_impl>>) {
756-
// This assumes that weak_ptr doesn't actually manage lifetime and
757-
// the object is guaranteed to be alive (which seems to be the
758-
// assumption across all graph code).
759-
return *Elem.lock();
760-
} else if constexpr (std::is_same_v<Ty, node>) {
761-
return *getSyclObjImpl(Elem);
762-
} else {
763-
return *Elem;
764-
}
765-
}
766-
};
767-
768752
template <typename... ContainerTy>
769753
using nodes_iterator_impl =
770-
variadic_iterator<nodes_deref_impl, node,
771-
typename ContainerTy::const_iterator...>;
754+
variadic_iterator<node, typename ContainerTy::const_iterator...>;
772755

773756
using nodes_iterator = nodes_iterator_impl<
774757
std::vector<std::shared_ptr<node_impl>>, std::vector<node_impl *>,
775-
// Next one is temporary. It looks like `weak_ptr`s aren't
776-
// used for the actual lifetime management and the objects are
777-
// always guaranteed to be alive. Once the code is cleaned
778-
// from `weak_ptr`s this alternative should be removed too.
779-
std::vector<std::weak_ptr<node_impl>>,
780-
//
781758
std::set<std::shared_ptr<node_impl>>, std::set<node_impl *>,
782-
//
783759
std::list<node_impl *>, std::vector<node>>;
784760

785761
class nodes_range : public iterator_range<nodes_iterator> {

sycl/source/detail/helpers.hpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,21 +35,19 @@ const RTDeviceBinaryImage *
3535
retrieveKernelBinary(queue_impl &Queue, KernelNameStrRefT KernelName,
3636
CGExecKernel *CGKernel = nullptr);
3737

38-
template <typename DereferenceImpl, typename SyclTy, typename... Iterators>
39-
class variadic_iterator {
38+
template <typename SyclTy, typename... Iterators> class variadic_iterator {
4039
using storage_iter = std::variant<Iterators...>;
4140

4241
storage_iter It;
4342

4443
public:
4544
using iterator_category = std::forward_iterator_tag;
4645
using difference_type = std::ptrdiff_t;
47-
using reference = decltype(DereferenceImpl::dereference(
48-
*std::declval<nth_type_t<0, Iterators...>>()));
49-
using value_type = std::remove_reference_t<reference>;
46+
using value_type = std::remove_reference_t<decltype(*getSyclObjImpl(
47+
std::declval<SyclTy>()))>;
5048
using sycl_type = SyclTy;
5149
using pointer = value_type *;
52-
static_assert(std::is_same_v<reference, value_type &>);
50+
using reference = value_type &;
5351

5452
variadic_iterator(const variadic_iterator &) = default;
5553
variadic_iterator(variadic_iterator &&) = default;
@@ -79,7 +77,16 @@ class variadic_iterator {
7977
decltype(auto) operator*() {
8078
return std::visit(
8179
[](auto &&It) -> decltype(auto) {
82-
return DereferenceImpl::dereference(*It);
80+
decltype(auto) Elem = *It;
81+
using Ty = std::decay_t<decltype(Elem)>;
82+
static_assert(!std::is_same_v<Ty, decltype(Elem)>);
83+
if constexpr (std::is_same_v<Ty, sycl_type>) {
84+
return *getSyclObjImpl(Elem);
85+
} else if constexpr (std::is_same_v<Ty, value_type>) {
86+
return Elem;
87+
} else {
88+
return *Elem;
89+
}
8390
},
8491
It);
8592
}

0 commit comments

Comments
 (0)