Ensure that Pair and Vec are trivial classes.

For std::copy to optimize a copy to memcpy, the valuetype must be both
trivially constructable and trivially copyable.

The new copy benchmarks highlighted an issue that std::copy'ing pairs
and vecs were not optimized to memcpy. For a 256 MiB buffer on my
laptop w/ GCC, the serial copy speeds were:

UInt8:                 10.10 GiB/s
Vec<UInt8, 2>           3.12 GiB/s
Pair<UInt32, Float32>   6.92 GiB/s

After this patch, the optimization occurs and a bitwise copy occurs:

UInt8:                 10.12 GiB/s
Vec<UInt8, 2>           9.66 GiB/s
Pair<UInt32, Float32>   9.88 GiB/s

Check were also added to the Vec and Pair unit tests to ensure that
this classes continue to be trivial.

The ArrayHandleSwizzle test was refactored a bit to eliminate a new
'possibly uninitialized memory' warning introduced with the default
Vec ctors.
This commit is contained in:
Allison Vacanti 2017-10-18 13:41:48 -04:00
parent d465d03047
commit 4cd791932b
5 changed files with 28 additions and 21 deletions

@ -65,11 +65,7 @@ struct Pair
SecondType second;
VTKM_EXEC_CONT
Pair()
: first()
, second()
{
}
Pair() = default;
VTKM_EXEC_CONT
Pair(const FirstType& firstSrc, const SecondType& secondSrc)
@ -93,12 +89,8 @@ struct Pair
}
VTKM_EXEC_CONT
vtkm::Pair<FirstType, SecondType>& operator=(const vtkm::Pair<FirstType, SecondType>& src)
{
this->first = src.first;
this->second = src.second;
return *this;
}
vtkm::Pair<FirstType, SecondType>& operator=(const vtkm::Pair<FirstType, SecondType>& src) =
default;
VTKM_EXEC_CONT
bool operator==(const vtkm::Pair<FirstType, SecondType>& other) const

@ -438,7 +438,7 @@ public:
protected:
VTKM_EXEC_CONT
VecBaseCommon() {}
VecBaseCommon() = default;
VTKM_EXEC_CONT
const DerivedClass& Derived() const { return *static_cast<const DerivedClass*>(this); }
@ -666,7 +666,7 @@ public:
protected:
VTKM_EXEC_CONT
VecBase() {}
VecBase() = default;
VTKM_EXEC_CONT
explicit VecBase(const ComponentType& value)
@ -824,7 +824,7 @@ public:
static const vtkm::IdComponent NUM_COMPONENTS = Size;
#endif
VTKM_EXEC_CONT Vec() {}
VTKM_EXEC_CONT Vec() = default;
VTKM_EXEC_CONT explicit Vec(const T& value)
: Superclass(value)
{
@ -851,7 +851,7 @@ public:
using ComponentType = T;
static const vtkm::IdComponent NUM_COMPONENTS = 0;
VTKM_EXEC_CONT Vec() {}
VTKM_EXEC_CONT Vec() = default;
VTKM_EXEC_CONT explicit Vec(const ComponentType&) {}
template <typename OtherType>
@ -882,7 +882,7 @@ class VTKM_ALWAYS_EXPORT Vec<T, 1> : public detail::VecBase<T, 1, Vec<T, 1>>
using Superclass = detail::VecBase<T, 1, Vec<T, 1>>;
public:
VTKM_EXEC_CONT Vec() {}
VTKM_EXEC_CONT Vec() = default;
VTKM_EXEC_CONT explicit Vec(const T& value)
: Superclass(value)
{
@ -912,7 +912,7 @@ class VTKM_ALWAYS_EXPORT Vec<T, 2> : public detail::VecBase<T, 2, Vec<T, 2>>
using Superclass = detail::VecBase<T, 2, Vec<T, 2>>;
public:
VTKM_EXEC_CONT Vec() {}
VTKM_EXEC_CONT Vec() = default;
VTKM_EXEC_CONT explicit Vec(const T& value)
: Superclass(value)
{
@ -941,7 +941,7 @@ class VTKM_ALWAYS_EXPORT Vec<T, 3> : public detail::VecBase<T, 3, Vec<T, 3>>
using Superclass = detail::VecBase<T, 3, Vec<T, 3>>;
public:
VTKM_EXEC_CONT Vec() {}
VTKM_EXEC_CONT Vec() = default;
VTKM_EXEC_CONT explicit Vec(const T& value)
: Superclass(value)
{
@ -972,7 +972,7 @@ class VTKM_ALWAYS_EXPORT Vec<T, 4> : public detail::VecBase<T, 4, Vec<T, 4>>
using Superclass = detail::VecBase<T, 4, Vec<T, 4>>;
public:
VTKM_EXEC_CONT Vec() {}
VTKM_EXEC_CONT Vec() = default;
VTKM_EXEC_CONT explicit Vec(const T& value)
: Superclass(value)
{

@ -141,12 +141,12 @@ struct SwizzleTests
auto refPortal = this->RefArray.GetPortalConstControl();
auto testPortal = testArray.GetPortalConstControl();
SwizzleVectorType refVecSwizzle(vtkm::TypeTraits<SwizzleVectorType>::ZeroInitialization());
for (vtkm::Id i = 0; i < testArray.GetNumberOfValues(); ++i)
{
// Manually swizzle the reference vector using the runtime map information:
ReferenceVectorType refVec = refPortal.Get(i);
SwizzleVectorType refVecSwizzle;
// Manually swizzle the reference vector using the runtime map information:
for (size_t j = 0; j < map.size(); ++j)
{
refVecSwizzle[static_cast<vtkm::IdComponent>(j)] = refVec[map[j]];

@ -31,6 +31,16 @@ namespace
template <typename T, typename U>
void PairTest()
{
{
using P = vtkm::Pair<T, U>;
// Pair types should preserve the trivial properties of their components.
// This insures that algorithms like std::copy will optimize fully.
VTKM_TEST_ASSERT(std::is_trivial<T>::value &&
std::is_trivial<U>::value == std::is_trivial<P>::value,
"PairType's triviality differs from ComponentTypes.");
}
//test that all the constructors work properly
{
vtkm::Pair<T, U> no_params_pair;

@ -323,6 +323,11 @@ void GeneralVecTypeTest(const vtkm::Vec<ComponentType, Size>&)
typedef vtkm::Vec<ComponentType, Size> T;
// Vector types should preserve the trivial properties of their components.
// This insures that algorithms like std::copy will optimize fully.
VTKM_TEST_ASSERT(std::is_trivial<ComponentType>::value == std::is_trivial<T>::value,
"VectorType's triviality differs from ComponentType.");
VTKM_TEST_ASSERT(T::NUM_COMPONENTS == Size, "NUM_COMPONENTS is wrong size.");
//grab the number of elements of T