From 7811cc4b1ec0106ca35ea8304144659052fdf45a Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Mon, 7 Dec 2020 15:19:24 -0700 Subject: [PATCH] Add standard support for read-only storage Many of the fancy `ArrayHandle`s are read-only and therefore connot really create write portals. Likewise, many `ArrayHandle`s (both read- only and read/write) have no way to resize themselves. In this case, implementing the `CreateWritePortal` and `ResizeBuffers` methods in the `Storage` class was troublesome. Mostly they just threw an exception, but they also sometimes had to deal with cases where the behavior was allowed. To simplify code for developers, this introduces a pair of macros: `VTKM_STORAGE_NO_RESIZE` and `VTKM_STORAGE_NO_WRITE_PORTAL`. These can be declared in a `Storage` implementation when the storage has no viable way to resize itself and create a write portal, respectively. Having boilerplate code for these methods also helps work around expected behavior for `ResizeBuffers`. `ResizeBuffers` should silently work when resizing to the same size. Also `ResizeBuffers` should behave well when resizing to 0 as that is what `ReleaseResources` does. --- vtkm/cont/ArrayHandleCartesianProduct.h | 18 +------ vtkm/cont/ArrayHandleImplicit.h | 30 ++--------- vtkm/cont/ArrayHandleTransform.h | 33 ++----------- vtkm/cont/CMakeLists.txt | 1 + vtkm/cont/Storage.cxx | 45 +++++++++++++++++ vtkm/cont/Storage.h | 52 +++++++++++++++++++- vtkm/cont/testing/TestingFancyArrayHandles.h | 5 +- vtkm/internal/ArrayPortalDummy.h | 41 +++++++++++++++ vtkm/internal/CMakeLists.txt | 1 + 9 files changed, 149 insertions(+), 77 deletions(-) create mode 100644 vtkm/cont/Storage.cxx create mode 100644 vtkm/internal/ArrayPortalDummy.h diff --git a/vtkm/cont/ArrayHandleCartesianProduct.h b/vtkm/cont/ArrayHandleCartesianProduct.h index b285b7918..e88debd2d 100644 --- a/vtkm/cont/ArrayHandleCartesianProduct.h +++ b/vtkm/cont/ArrayHandleCartesianProduct.h @@ -221,6 +221,8 @@ class Storage, vtkm::cont::StorageTagCartesianProduct, typename Storage1::ReadPortalType, @@ -245,22 +247,6 @@ public: Storage3::GetNumberOfValues(Buffers3(buffers))); } - VTKM_CONT static void ResizeBuffers(vtkm::Id numValues, - vtkm::cont::internal::Buffer* buffers, - vtkm::CopyFlag vtkmNotUsed(preserve), - vtkm::cont::Token& vtkmNotUsed(token)) - { - if (numValues == GetNumberOfValues(buffers)) - { - // In general, we don't allow resizing of the array, but if it was "allocated" to the - // correct size, we will allow that. - } - else - { - throw vtkm::cont::ErrorBadAllocation("Does not make sense."); - } - } - VTKM_CONT static ReadPortalType CreateReadPortal(const vtkm::cont::internal::Buffer* buffers, vtkm::cont::DeviceAdapterId device, vtkm::cont::Token& token) diff --git a/vtkm/cont/ArrayHandleImplicit.h b/vtkm/cont/ArrayHandleImplicit.h index 87fda7b80..099713e7b 100644 --- a/vtkm/cont/ArrayHandleImplicit.h +++ b/vtkm/cont/ArrayHandleImplicit.h @@ -97,11 +97,10 @@ struct VTKM_ALWAYS_EXPORT { VTKM_IS_TRIVIALLY_COPYABLE(ArrayPortalType); - using ReadPortalType = ArrayPortalType; + VTKM_STORAGE_NO_RESIZE; + VTKM_STORAGE_NO_WRITE_PORTAL; - // Note that this portal is almost certainly read-only, so you will probably get - // an error if you try to write to it. - using WritePortalType = ArrayPortalType; + using ReadPortalType = ArrayPortalType; // Implicit array has one buffer that should be empty (NumberOfBytes = 0), but holds // the metadata for the array. @@ -112,35 +111,12 @@ struct VTKM_ALWAYS_EXPORT return buffers[0].GetMetaData().GetNumberOfValues(); } - VTKM_CONT static void ResizeBuffers(vtkm::Id numValues, - vtkm::cont::internal::Buffer* buffers, - vtkm::CopyFlag, - vtkm::cont::Token&) - { - if (numValues == GetNumberOfValues(buffers)) - { - // In general, we don't allow resizing of the array, but if it was "allocated" to the - // correct size, we will allow that. - } - else - { - throw vtkm::cont::ErrorBadAllocation("Cannot allocate/resize implicit arrays."); - } - } - VTKM_CONT static ReadPortalType CreateReadPortal(const vtkm::cont::internal::Buffer* buffers, vtkm::cont::DeviceAdapterId, vtkm::cont::Token&) { return buffers[0].GetMetaData(); } - - VTKM_CONT static WritePortalType CreateWritePortal(const vtkm::cont::internal::Buffer*, - vtkm::cont::DeviceAdapterId, - vtkm::cont::Token&) - { - throw vtkm::cont::ErrorBadAllocation("Cannot write to implicit arrays."); - } }; /// Given an array portal, returns the buffers for the `ArrayHandle` with a storage that diff --git a/vtkm/cont/ArrayHandleTransform.h b/vtkm/cont/ArrayHandleTransform.h index 5adcc0bfb..cd8d4a970 100644 --- a/vtkm/cont/ArrayHandleTransform.h +++ b/vtkm/cont/ArrayHandleTransform.h @@ -252,16 +252,14 @@ class Storage::ValueT static constexpr vtkm::IdComponent NUM_METADATA_BUFFERS = 1; public: + VTKM_STORAGE_NO_RESIZE; + VTKM_STORAGE_NO_WRITE_PORTAL; + using ReadPortalType = vtkm::internal::ArrayPortalTransform; - // Note that this array is read only, so you really should only be getting the const - // version of the portal. If you actually try to write to this portal, you will - // get an error. - using WritePortalType = ReadPortalType; - VTKM_CONT static vtkm::IdComponent GetNumberOfBuffers() { return SourceStorage::GetNumberOfBuffers() + NUM_METADATA_BUFFERS; @@ -272,23 +270,6 @@ public: return SourceStorage::GetNumberOfValues(buffers + NUM_METADATA_BUFFERS); } - VTKM_CONT static void ResizeBuffers(vtkm::Id numValues, - vtkm::cont::internal::Buffer* buffers, - vtkm::CopyFlag vtkmNotUsed(preserve), - vtkm::cont::Token& vtkmNotUsed(token)) - { - if (numValues == GetNumberOfValues(buffers)) - { - // In general, we don't allow resizing of the array, but if it was "allocated" to the - // correct size, we will allow that. - } - else - { - throw vtkm::cont::ErrorBadAllocation( - "ArrayHandleTransform is read only. It cannot be allocated."); - } - } - VTKM_CONT static ReadPortalType CreateReadPortal(const vtkm::cont::internal::Buffer* buffers, vtkm::cont::DeviceAdapterId device, vtkm::cont::Token& token) @@ -307,14 +288,6 @@ public: } } - VTKM_CONT static WritePortalType CreateWritePortal(vtkm::cont::internal::Buffer*, - vtkm::cont::DeviceAdapterId, - vtkm::cont::Token&) - { - throw vtkm::cont::ErrorBadType( - "ArrayHandleTransform is read only. Cannot get writable portal."); - } - VTKM_CONT static std::vector CreateBuffers( const ArrayHandleType& handle, const FunctorType& functor = FunctorType()) diff --git a/vtkm/cont/CMakeLists.txt b/vtkm/cont/CMakeLists.txt index 0e7bade2b..5880decb0 100644 --- a/vtkm/cont/CMakeLists.txt +++ b/vtkm/cont/CMakeLists.txt @@ -151,6 +151,7 @@ set(sources Initialize.cxx Logging.cxx RuntimeDeviceTracker.cxx + Storage.cxx Token.cxx TryExecute.cxx ) diff --git a/vtkm/cont/Storage.cxx b/vtkm/cont/Storage.cxx new file mode 100644 index 000000000..f734d66f3 --- /dev/null +++ b/vtkm/cont/Storage.cxx @@ -0,0 +1,45 @@ +//============================================================================ +// Copyright (c) Kitware, Inc. +// All rights reserved. +// See LICENSE.txt for details. +// +// This software is distributed WITHOUT ANY WARRANTY; without even +// the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR +// PURPOSE. See the above copyright notice for more information. +//============================================================================ + +#include + +namespace vtkm +{ +namespace cont +{ +namespace internal +{ +namespace detail +{ + +void StorageNoResizeImpl(vtkm::Id currentNumValues, + vtkm::Id requestedNumValues, + std::string storageTagName) +{ + if (currentNumValues == requestedNumValues) + { + // Array resized to current size. This is OK. + } + else if (requestedNumValues == 0) + { + // Array resized to zero. This can happen when releasing resources. + // Should we try to clear out the buffers, or avoid that for messing up shared buffers? + } + else + { + throw vtkm::cont::ErrorBadAllocation("Cannot resize arrays with storage type of " + + storageTagName); + } +} + +} +} +} +} // namespace vtkm::cont::internal::detail diff --git a/vtkm/cont/Storage.h b/vtkm/cont/Storage.h index 11e1c9a33..c00b9f3fb 100644 --- a/vtkm/cont/Storage.h +++ b/vtkm/cont/Storage.h @@ -18,10 +18,16 @@ #define VTKM_STORAGE VTKM_STORAGE_BASIC #endif +#include #include -#include -#include +#include + +#include +#include +#include + +#include namespace vtkm { @@ -173,6 +179,48 @@ public: }; #endif // VTKM_DOXYGEN_ONLY +namespace detail +{ + +VTKM_CONT_EXPORT void StorageNoResizeImpl(vtkm::Id currentNumValues, + vtkm::Id requestedNumValues, + std::string storageTagName); + +} // namespace detail + +template +struct StorageTraits; + +template +struct StorageTraits> +{ + using ValueType = T; + using Tag = S; +}; + +#define VTKM_STORAGE_NO_RESIZE \ + VTKM_CONT static void ResizeBuffers( \ + vtkm::Id numValues, vtkm::cont::internal::Buffer* buffers, vtkm::CopyFlag, vtkm::cont::Token&) \ + { \ + vtkm::cont::internal::detail::StorageNoResizeImpl( \ + GetNumberOfValues(buffers), \ + numValues, \ + vtkm::cont::TypeToString::Tag>()); \ + } \ + using ResizeBuffersEatComma = void + +#define VTKM_STORAGE_NO_WRITE_PORTAL \ + using WritePortalType = vtkm::internal::ArrayPortalDummy< \ + typename vtkm::cont::internal::StorageTraits::ValueType>; \ + VTKM_CONT static WritePortalType CreateWritePortal( \ + vtkm::cont::internal::Buffer*, vtkm::cont::DeviceAdapterId, vtkm::cont::Token&) \ + { \ + throw vtkm::cont::ErrorBadAllocation( \ + "Cannot write to arrays with storage type of " + \ + vtkm::cont::TypeToString::Tag>()); \ + } \ + using CreateWritePortalEatComma = void + } // namespace internal } } // namespace vtkm::cont diff --git a/vtkm/cont/testing/TestingFancyArrayHandles.h b/vtkm/cont/testing/TestingFancyArrayHandles.h index 0f80efa1a..adb398e07 100644 --- a/vtkm/cont/testing/TestingFancyArrayHandles.h +++ b/vtkm/cont/testing/TestingFancyArrayHandles.h @@ -343,8 +343,6 @@ private: vtkm::cont::ArrayCopy(soaArray, basicArray); VTKM_TEST_ASSERT(basicArray.GetNumberOfValues() == ARRAY_SIZE); CheckPortal(basicArray.ReadPortal()); - - soaArray.ReleaseResources(); } { @@ -373,6 +371,9 @@ private: vtkm::cont::make_ArrayHandleSOA(vtkm::CopyFlag::Off, vector0, vector1, vector2); VTKM_TEST_ASSERT(soaArray.GetNumberOfValues() == ARRAY_SIZE); CheckPortal(soaArray.ReadPortal()); + + // Make sure calling ReleaseResources does not result in error. + soaArray.ReleaseResources(); } { diff --git a/vtkm/internal/ArrayPortalDummy.h b/vtkm/internal/ArrayPortalDummy.h new file mode 100644 index 000000000..79ffcf0f6 --- /dev/null +++ b/vtkm/internal/ArrayPortalDummy.h @@ -0,0 +1,41 @@ +//============================================================================ +// Copyright (c) Kitware, Inc. +// All rights reserved. +// See LICENSE.txt for details. +// +// This software is distributed WITHOUT ANY WARRANTY; without even +// the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR +// PURPOSE. See the above copyright notice for more information. +//============================================================================ +#ifndef vtk_m_internal_ArrayPortalDummy +#define vtk_m_internal_ArrayPortalDummy + +#include +#include + +namespace vtkm +{ +namespace internal +{ + +/// A class that can be used in place of an `ArrayPortal` when the `ArrayPortal` is +/// not actually supported. It allows templates to be compiled, but will cause undefined +/// behavior if actually used. +template +struct ArrayPortalDummy +{ + using ValueType = T; + + VTKM_EXEC_CONT vtkm::Id GetNumberOfValues() const { return 0; } + + VTKM_EXEC_CONT ValueType Get(vtkm::Id) const + { + VTKM_ASSERT(false && "Tried to use a dummy portal."); + return ValueType{}; + } +}; + +} +} // namespace vtkm::internal + +#endif //vtk_m_internal_ArrayPortalDummy diff --git a/vtkm/internal/CMakeLists.txt b/vtkm/internal/CMakeLists.txt index 0a9ef5499..c13773c38 100755 --- a/vtkm/internal/CMakeLists.txt +++ b/vtkm/internal/CMakeLists.txt @@ -49,6 +49,7 @@ vtkm_install_headers(vtkm/internal set(headers ArrayPortalBasic.h + ArrayPortalDummy.h ArrayPortalHelpers.h ArrayPortalUniformPointCoordinates.h ArrayPortalValueReference.h