Skip to content

Remove nullptr conversions to/from ManagedArray #286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ The format of this file is based on [Keep a Changelog](http://keepachangelog.com
### Removed
- Removes deprecated ManagedArray::getPointer method. Use ManagedArray::data instead.
- Removes optional support for implicitly casting between raw pointers and ManagedArrays (CHAI\_ENABLE\_IMPLICIT\_CONVERSIONS). Use makeManagedArray and ManagedArray::data to perform explicit conversions instead.
- Removes equality and inequality comparison operators between ManagedArrays and raw pointers
- Removes equality and inequality comparison operators between ManagedArrays and raw pointers.
- Removes nullptr constructor, assignment operator, and comparison operators for ManagedArray.
- Removes move assignment operator.

## [Version 2024.07.0] - Release date 2024-07-26

Expand Down
20 changes: 3 additions & 17 deletions src/chai/ManagedArray.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,6 @@ class ManagedArray : public CHAICopyable
*/
CHAI_HOST_DEVICE ManagedArray(ManagedArray const& other);

/*!
* \brief Construct a ManagedArray from a nullptr.
*/
CHAI_HOST_DEVICE ManagedArray(std::nullptr_t other);

CHAI_HOST ManagedArray(PointerRecord* record, ExecutionSpace space);

/*!
Expand Down Expand Up @@ -254,18 +249,9 @@ class ManagedArray : public CHAICopyable

ManagedArray<T>& operator=(ManagedArray const & other) = default;

CHAI_HOST_DEVICE ManagedArray<T>& operator=(ManagedArray && other);

CHAI_HOST_DEVICE ManagedArray<T>& operator=(std::nullptr_t);


CHAI_HOST_DEVICE bool operator==(const ManagedArray<T>& rhs) const;
CHAI_HOST_DEVICE bool operator!=(const ManagedArray<T>& from) const;

CHAI_HOST_DEVICE bool operator==(std::nullptr_t from) const;
CHAI_HOST_DEVICE bool operator!=(std::nullptr_t from) const;


CHAI_HOST_DEVICE explicit operator bool() const;


Expand Down Expand Up @@ -387,14 +373,14 @@ class ManagedArray : public CHAICopyable
// shenanigan reasons need to be defined here.
#if !defined(CHAI_DISABLE_RM)
// if T is a CHAICopyable, then it is important to initialize all the
// ManagedArrays to nullptr at allocation, since it is extremely easy to
// trigger a moveInnerImpl, which expects inner values to be initialized.
// ManagedArrays at allocation, since it is extremely easy to trigger
// a moveInnerImpl, which expects inner values to be initialized.
template <bool B = std::is_base_of<CHAICopyable, T>::value,
typename std::enable_if<B, int>::type = 0>
CHAI_HOST bool initInner(size_t start = 0)
{
for (size_t i = start; i < m_size/sizeof(T); ++i) {
m_active_base_pointer[i] = nullptr;
m_active_base_pointer[i] = T();
}
return true;
}
Expand Down
60 changes: 4 additions & 56 deletions src/chai/ManagedArray.inl
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,6 @@ ManagedArray<T>::ManagedArray(
this->allocate(elems, space);
}

template<typename T>
CHAI_INLINE
CHAI_HOST_DEVICE ManagedArray<T>::ManagedArray(std::nullptr_t) :
ManagedArray()
{
}

template<typename T>
CHAI_INLINE
CHAI_HOST ManagedArray<T>::ManagedArray(PointerRecord* record, ExecutionSpace space):
Expand Down Expand Up @@ -181,8 +174,8 @@ CHAI_HOST void ManagedArray<T>::allocate(
m_active_pointer = m_active_base_pointer; // Cannot be a slice

// if T is a CHAICopyable, then it is important to initialize all the
// ManagedArrays to nullptr at allocation, since it is extremely easy to
// trigger a moveInnerImpl, which expects inner values to be initialized.
// ManagedArrays at allocation, since it is extremely easy to trigger
// a moveInnerImpl, which expects inner values to be initialized.
initInner();

#if defined(CHAI_ENABLE_UM)
Expand Down Expand Up @@ -235,8 +228,8 @@ CHAI_HOST void ManagedArray<T>::reallocate(size_t elems)
m_active_pointer = m_active_base_pointer; // Cannot be a slice

// if T is a CHAICopyable, then it is important to initialize all the new
// ManagedArrays to nullptr at allocation, since it is extremely easy to
// trigger a moveInnerImpl, which expects inner values to be initialized.
// ManagedArrays at allocation, since it is extremely easy to trigger a
// moveInnerImpl, which expects inner values to be initialized.
if (initInner(old_size/sizeof(T))) {
// if we are active on the GPU, we need to send any newly initialized inner members to the device
if (m_pointer_record->m_last_space == GPU && old_size < m_size) {
Expand Down Expand Up @@ -566,36 +559,6 @@ typename std::enable_if< !std::is_const<U>::value ,
return *reinterpret_cast<ManagedArray<const T> *>(const_cast<ManagedArray<T> *>(this));
}

template<typename T>
CHAI_INLINE
CHAI_HOST_DEVICE
ManagedArray<T>&
ManagedArray<T>::operator= (ManagedArray && other) {
*this = other;
other = nullptr;
return *this;
}

template<typename T>
CHAI_INLINE
CHAI_HOST_DEVICE
ManagedArray<T>&
ManagedArray<T>::operator= (std::nullptr_t) {
m_active_pointer = nullptr;
m_active_base_pointer = nullptr;
m_size = 0;
m_offset = 0;
#if !defined(CHAI_DEVICE_COMPILE)
m_pointer_record = &ArrayManager::s_null_record;
m_resource_manager = ArrayManager::getInstance();
#else
m_pointer_record = nullptr;
m_resource_manager = nullptr;
#endif
m_is_slice = false;
return *this;
}

template<typename T>
CHAI_INLINE
CHAI_HOST_DEVICE
Expand All @@ -614,21 +577,6 @@ ManagedArray<T>::operator!= (const ManagedArray<T>& rhs) const
return (m_active_pointer != rhs.m_active_pointer);
}

template<typename T>
CHAI_INLINE
CHAI_HOST_DEVICE
bool
ManagedArray<T>::operator== (std::nullptr_t from) const {
return m_active_pointer == from || m_size == 0;
}
template<typename T>
CHAI_INLINE
CHAI_HOST_DEVICE
bool
ManagedArray<T>::operator!= (std::nullptr_t from) const {
return m_active_pointer != from && m_size > 0;
}

template<typename T>
CHAI_INLINE
CHAI_HOST_DEVICE
Expand Down
47 changes: 0 additions & 47 deletions src/chai/ManagedArray_thin.inl
Original file line number Diff line number Diff line change
Expand Up @@ -59,20 +59,6 @@ CHAI_HOST_DEVICE ManagedArray<T>::ManagedArray(size_t elems, ExecutionSpace spac
#endif
}


template <typename T>
CHAI_INLINE CHAI_HOST_DEVICE ManagedArray<T>::ManagedArray(std::nullptr_t)
: m_active_pointer(nullptr),
m_active_base_pointer(nullptr),
m_resource_manager(nullptr),
m_size(0),
m_offset(0),
m_pointer_record(nullptr),
m_is_slice(false)
{
}


template<typename T>
CHAI_INLINE
CHAI_HOST ManagedArray<T>::ManagedArray(PointerRecord* record, ExecutionSpace space):
Expand Down Expand Up @@ -363,28 +349,6 @@ ManagedArray<T>::operator typename std::
nullptr);
}

template<typename T>
CHAI_INLINE
CHAI_HOST_DEVICE
ManagedArray<T>&
ManagedArray<T>::operator= (ManagedArray && other) {
if (this != &other) {
*this = other;
other = nullptr;
}
return *this;
}

template <typename T>
CHAI_INLINE CHAI_HOST_DEVICE ManagedArray<T>& ManagedArray<T>::operator=(std::nullptr_t from)
{
m_active_pointer = from;
m_active_base_pointer = from;
m_size = 0;
m_is_slice = false;
return *this;
}

template <typename T>
CHAI_INLINE CHAI_HOST_DEVICE bool ManagedArray<T>::operator==(
const ManagedArray<T>& rhs) const
Expand All @@ -399,17 +363,6 @@ CHAI_INLINE CHAI_HOST_DEVICE bool ManagedArray<T>::operator!=(
return (m_active_pointer != rhs.m_active_pointer);
}

template <typename T>
CHAI_INLINE CHAI_HOST_DEVICE bool ManagedArray<T>::operator==( std::nullptr_t from) const
{
return m_active_pointer == from;
}
template <typename T>
CHAI_INLINE CHAI_HOST_DEVICE bool ManagedArray<T>::operator!=( std::nullptr_t from) const
{
return m_active_pointer != from;
}

template <typename T>
CHAI_INLINE CHAI_HOST_DEVICE ManagedArray<T>::operator bool() const
{
Expand Down
19 changes: 0 additions & 19 deletions tests/integration/managed_array_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -755,25 +755,6 @@ GPU_TEST(ManagedArray, ReallocateGPU)
}
#endif

TEST(ManagedArray, NullpointerConversions)
{
chai::ManagedArray<float> a;
a.free();
a = nullptr;

chai::ManagedArray<const float> b;
b.free();
b = nullptr;

ASSERT_EQ(a.size(), 0u);
ASSERT_EQ(b.size(), 0u);

chai::ManagedArray<float> c(nullptr);

ASSERT_EQ(c.size(), 0u);
assert_empty_map(true);
}

TEST(ManagedArray, PodTest)
{
chai::ManagedArray<my_point> array(1);
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/managed_ptr_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class TestBase {

class TestDerived : public TestBase {
public:
CHAI_HOST_DEVICE TestDerived() : TestBase(), m_values(nullptr) {}
CHAI_HOST_DEVICE TestDerived() : TestBase(), m_values() {}
CHAI_HOST_DEVICE TestDerived(chai::ManagedArray<int> values) : TestBase(), m_values(values) {}
CHAI_HOST_DEVICE virtual ~TestDerived() {}

Expand Down