device_raw_ptr
is a simple fancy pointer. It essentially wrap pointers to GPU memory. It's sole purpose is to separate out host pointers from device pointers, i.e. they should not be inter-convertible and must not be dereferenced. At the same time, they should be zero-cost (with respect to raw host pointers) and be maximally compatible with regular pointers.
Helper class:
template <class T>
struct equality_operators {
/*
** The deriving class must implement the following:
** friend bool operator==(const T&, const T&);
*/
friend bool operator!=(const T& lhs, const T& rhs) { return !static_cast<bool>(lhs == rhs); }
};
template <class T>
struct less_than_operators {
/*
** The deriving class must implement the following:
** friend bool operator<(const T&, const T&);
*/
friend bool operator>(const T& lhs, const T& rhs) { return rhs < lhs; }
friend bool operator<=(const T& lhs, const T& rhs) { return !static_cast<bool>(lhs > rhs); }
friend bool operator>=(const T& lhs, const T& rhs) { return !static_cast<bool>(lhs < rhs); }
};
template <class T>
struct relational_operators : equality_operators<T>, less_than_operators<T> { };
device_raw_ptr
implementation:
template <class T>
class device_raw_ptr : public relational_operators<T> {
static_assert(std::is_standard_layout<T>::value, "T must satisfy StandardLayoutType");
public:
using element_type = std::remove_extent_t<T>;
constexpr device_raw_ptr() noexcept = default;
constexpr device_raw_ptr(std::nullptr_t) noexcept : ptr { nullptr } { }
constexpr device_raw_ptr(const device_raw_ptr& other) noexcept = default;
explicit device_raw_ptr(element_type* ptr_) noexcept : ptr{ ptr_ } { }
device_raw_ptr(device_raw_ptr&& other) noexcept : ptr{ other.ptr } { other.reset(); }
device_raw_ptr& operator=(const device_raw_ptr& other) noexcept {
swap(device_raw_ptr(other), *this);
return *this;
}
device_raw_ptr& operator=(device_raw_ptr&& other) noexcept {
swap(device_raw_ptr(other), *this);
return *this;
}
void reset() noexcept { ptr = nullptr; }
void reset(T* ptr_) noexcept { ptr = ptr_; }
element_type* get() noexcept { return ptr; };
const element_type* get() const noexcept { return ptr; }
friend void swap(device_raw_ptr& lhs, device_raw_ptr& rhs) noexcept {
using std::swap;
std::swap(lhs.ptr, rhs.ptr);
}
explicit operator bool() const noexcept { return static_cast<bool>(ptr); }
device_raw_ptr& operator++() noexcept {
++ptr;
return *this;
}
device_raw_ptr operator++(int) noexcept {
device_raw_ptr tmp(*this);
ptr++;
return tmp;
}
device_raw_ptr& operator+=(std::ptrdiff_t offset) noexcept {
ptr += offset;
return *this;
}
device_raw_ptr& operator-=(std::ptrdiff_t offset) noexcept {
ptr -= offset;
return *this;
}
friend device_raw_ptr& operator+(device_raw_ptr lhs, std::ptrdiff_t offset) noexcept {
lhs += offset;
return lhs;
}
friend device_raw_ptr& operator-(device_raw_ptr lhs, std::ptrdiff_t offset) noexcept {
lhs -= offset;
return lhs;
}
/* required by relational_operators base class */
friend bool operator==(const device_raw_ptr& lhs, const device_raw_ptr& rhs) noexcept { return lhs.ptr == rhs.ptr; }
friend bool operator<(const device_raw_ptr& lhs, const device_raw_ptr& rhs) noexcept { return lhs.ptr < rhs.ptr; }
protected:
T *ptr;
};
template <class T, class U, class V>
std::basic_ostream<U, V>& operator<<(std::basic_ostream<U, V>& os, const device_raw_ptr<T>& other) {
os << other.get() << " (device)";
return os;
}
I am also looking for suggestions on how to order different things inside a class.
Might it be better to initialize with nullptr
instead of the default constructor. This breaks compatibility but I think it might be worth considering the compiler would mostly optimize the assignment if it's immediately overwritten.
2 Answers 2
device_raw_ptr
is extremely cheap to copy, so remove all hints of move-semantics and use ofswap()
.Now you can remove the copy-constructor and copy-assignment, as there is no need to explicitly declare them. Especially making them user-defined must be avoided to keep them trivial.
Kudos on trying to use the approved two-step for
swap()
. Though you get a failing grade anyway because you bungled it by using a qualified call in the second part. Hopefully, C++20 will abolish that nonsense by introducing customization point objects.Yes, you should pass your
device_raw_ptr
by value if you have the choice, as it is a tiny trivial type. Still, refrain from returning a reference to such a temporary.There is no reason
operator-(device_raw_ptr, std::ptrdiff_t)
should not beconstexpr
. Aside from your implementation for some reason delegating tooperator-=
, which is not. Same foroperator+
which usesoperator+=
.Is there any reason you don't support subtracting a
device_raw_ptr
from another?I'm really puzzled why you make the only data-member
protected
instead ofprivate
.
-
\$\begingroup\$ Wouldn't the implicitly defined copy-assignment operator return a reference? \$\endgroup\$Yashas– Yashas2019年04月08日 05:05:23 +00:00Commented Apr 8, 2019 at 5:05
-
\$\begingroup\$ I have incorporated the changes you suggested. Please check latest code \$\endgroup\$Yashas– Yashas2019年04月08日 07:43:58 +00:00Commented Apr 8, 2019 at 7:43
Helper classes
There is no need to use static_cast<bool>
for your comparisons. The relational operators are already bool
values. (If they are not, that is a problem with the definition of the operator for the type T
.)
The standard <utility>
header provides definitions for operator!=
(from operator==
) and operator>
, operator<=
, and operator>=
(from operator<
). There is no need for you to define those four operators if you have the other two (equality and less-than).
Why do you have the relational_operators
struct at all? It shouldn't be necessary.
Implementation
The default constructor for device_raw_ptr
leaves the ptr
member uninitialized. Typically a class like this would initialize ptr
to nullptr
, and you wouldn't need the constructor that takes a std::nullptr_t
object.
The copy assignment operator should just be ptr = other.ptr
, since that is the only thing in your class. The way you have it is nonstandard behavior. You construct a temporary, then pass it as a non-const reference to swap
. This is not supported as part of the language, although some compilers (MSVC) support it as an extension. You're constructing a temporary, doing a swap, then destroying the temporary (a noop in this case). Similarly, the move assignment operator can be simplified to not use the temporary (ptr = other.ptr; other.reset();
, or use three statements with an assignment to a local to avoid problems if you move assign an object to itself).
operator bool
does not need a static_cast
. Perhaps an explicit ptr != nullptr
check, although a pointer will implicitly convert to a bool
.
-
\$\begingroup\$ I thought using
std::rel_ops
is generally frowned upon. stackoverflow.com/questions/6225375/idiomatic-use-of-stdrel-ops suggests usingboost:operators
. I wrote my own version instead. \$\endgroup\$Yashas– Yashas2019年04月07日 17:07:44 +00:00Commented Apr 7, 2019 at 17:07 -
\$\begingroup\$ The default constructor leaves
ptr
unitialized because that's how raw pointers behave (initialized with garbage?). But I am considering the option of initializing it tonullptr
. Thestd::nullptr_t
is a consequence of the aforementioned statement. I intended to have aconstexpr
constructor which setsptr
tonullptr
. \$\endgroup\$Yashas– Yashas2019年04月07日 17:09:16 +00:00Commented Apr 7, 2019 at 17:09 -
\$\begingroup\$ For the abuse of swap, I made started (codereview.stackexchange.com/questions/217019/…) using temporaries right after I posted the question. It was an attempt to reuse the constructors to perform move/copy but I think it was overkill for such a simple class. \$\endgroup\$Yashas– Yashas2019年04月07日 17:12:27 +00:00Commented Apr 7, 2019 at 17:12
-
\$\begingroup\$ @Yashas
std::rel_ops
is there to avoid having duplicate definitions of those relational operators. If a class does not want some of them (as mentioned in your linked question), then it should define all of them, and= delete
the ones it doesn't want to support. But its up to you to decide how to implement them. \$\endgroup\$1201ProgramAlarm– 1201ProgramAlarm2019年04月07日 17:14:05 +00:00Commented Apr 7, 2019 at 17:14 -
\$\begingroup\$ @Yashas leaving raw pointer uninitialized is, generally, a bad idea. The cost of assigning a
nullptr
someplace where it isn't strictly necessary is outweighed by the predictability and consistent (mis)behavior the NULL value will give you. \$\endgroup\$1201ProgramAlarm– 1201ProgramAlarm2019年04月07日 17:16:16 +00:00Commented Apr 7, 2019 at 17:16
swap
which accepts non-const lvalue as arguments. You can find it in the copy & move constructor. \$\endgroup\$relational_operator
inheritance. \$\endgroup\$