From 40c579f119f2b03b247c39a765fecea27384c9cd Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Mon, 19 May 2014 13:17:04 -0400 Subject: [PATCH 1/5] Correcting alignment issues so we compile on windows. --- vtkm/Types.h | 33 ++++++++++++------- vtkm/cont/ArrayContainerControlImplicit.h | 2 +- vtkm/cont/ArrayHandle.h | 2 +- .../internal/ArrayHandleExecutionManager.h | 4 +-- vtkm/cont/internal/ArrayManagerExecution.h | 2 +- .../ArrayManagerExecutionShareWithControl.h | 2 +- vtkm/cont/internal/ArrayPortalFromIterators.h | 2 +- vtkm/cont/internal/ArrayTransfer.h | 2 +- .../UnitTestArrayContainerControlBasic.cxx | 2 +- 9 files changed, 30 insertions(+), 21 deletions(-) diff --git a/vtkm/Types.h b/vtkm/Types.h index d13a9b186..d5f2049cb 100644 --- a/vtkm/Types.h +++ b/vtkm/Types.h @@ -107,6 +107,16 @@ typedef unsigned long long UInt64Type; #error Could not find a 64-bit integer. #endif +//for windows support we need a macro to wrap the alignment call, +//since for windows you need declspec(align) +#ifdef _MSC_VER +# define VTKM_ALIGN_BEGIN(size) __declspec(align(size)) +# define VTKM_ALIGN_END(size) +#else //we presume clang or gcc +# define VTKM_ALIGN_BEGIN(size) +# define VTKM_ALIGN_END(size) __attribute__((aligned(size))) +#endif + //----------------------------------------------------------------------------- template @@ -249,12 +259,12 @@ struct copy_vector<3> #if VTKM_SIZE_ID == 4 /// Represents an ID. -typedef internal::Int32Type Id __attribute__ ((aligned(VTKM_SIZE_ID))); +typedef internal::Int32Type Id; #elif VTKM_SIZE_ID == 8 /// Represents an ID. -typedef internal::Int64Type Id __attribute__ ((aligned(VTKM_SIZE_ID))); +typedef internal::Int64Type Id; #else #error Unknown Id Size @@ -263,12 +273,12 @@ typedef internal::Int64Type Id __attribute__ ((aligned(VTKM_SIZE_ID))); #ifdef VTKM_USE_DOUBLE_PRECISION /// Scalar corresponds to a floating point number. -typedef double Scalar __attribute__ ((aligned(VTKM_SIZE_SCALAR))); +typedef double Scalar; #else //VTKM_USE_DOUBLE_PRECISION /// Scalar corresponds to a floating point number. -typedef float Scalar __attribute__ ((aligned(VTKM_SIZE_SCALAR))); +typedef float Scalar; #endif //VTKM_USE_DOUBLE_PRECISION @@ -431,12 +441,13 @@ protected: }; /// Vector2 corresponds to a 2-tuple -typedef vtkm::Tuple - Vector2 __attribute__ ((aligned(VTKM_ALIGNMENT_TWO_SCALAR))); +namespace internal + { typedef VTKM_ALIGN_BEGIN(VTKM_ALIGNMENT_TWO_SCALAR) vtkm::Tuple Vector2 VTKM_ALIGN_END(VTKM_ALIGNMENT_TWO_SCALAR); } +typedef internal::Vector2 Vector2; /// Id2 corresponds to a 2-dimensional index -typedef vtkm::Tuple Id2 __attribute__ ((aligned(VTKM_SIZE_ID))); +typedef VTKM_ALIGN_BEGIN(VTKM_SIZE_ID) vtkm::Tuple Id2 VTKM_ALIGN_END(VTKM_SIZE_ID); template class Tuple @@ -511,8 +522,7 @@ protected: }; /// Vector3 corresponds to a 3-tuple -typedef vtkm::Tuple - Vector3 __attribute__ ((aligned(VTKM_SIZE_SCALAR))); +typedef VTKM_ALIGN_BEGIN(VTKM_SIZE_SCALAR) vtkm::Tuple Vector3 VTKM_ALIGN_END(VTKM_SIZE_SCALAR); template class Tuple @@ -592,13 +602,12 @@ protected: }; /// Vector4 corresponds to a 4-tuple -typedef vtkm::Tuple - Vector4 __attribute__ ((aligned(VTKM_ALIGNMENT_FOUR_SCALAR))); +typedef VTKM_ALIGN_BEGIN(VTKM_ALIGNMENT_FOUR_SCALAR) vtkm::Tuple Vector4 VTKM_ALIGN_END(VTKM_ALIGNMENT_FOUR_SCALAR); /// Id3 corresponds to a 3-dimensional index for 3d arrays. Note that /// the precision of each index may be less than vtkm::Id. -typedef vtkm::Tuple Id3 __attribute__ ((aligned(VTKM_SIZE_ID))); +typedef VTKM_ALIGN_BEGIN(VTKM_SIZE_ID) vtkm::Tuple Id3 VTKM_ALIGN_END(VTKM_SIZE_ID); /// Initializes and returns a Vector2. VTKM_EXEC_CONT_EXPORT vtkm::Vector2 make_Vector2(vtkm::Scalar x, diff --git a/vtkm/cont/ArrayContainerControlImplicit.h b/vtkm/cont/ArrayContainerControlImplicit.h index 4acb1c682..9287fe8f3 100644 --- a/vtkm/cont/ArrayContainerControlImplicit.h +++ b/vtkm/cont/ArrayContainerControlImplicit.h @@ -131,7 +131,7 @@ public: return this->Portal.GetNumberOfValues(); } - VTKM_CONT_EXPORT void LoadDataForInput(PortalConstControl portal) + VTKM_CONT_EXPORT void LoadDataForInput(const PortalConstControl& portal) { this->Portal = portal; this->PortalValid = true; diff --git a/vtkm/cont/ArrayHandle.h b/vtkm/cont/ArrayHandle.h index 1ddba28db..067bb38ed 100644 --- a/vtkm/cont/ArrayHandle.h +++ b/vtkm/cont/ArrayHandle.h @@ -101,7 +101,7 @@ public: /// Constructs an ArrayHandle pointing to the data in the given array portal. /// - VTKM_CONT_EXPORT ArrayHandle(PortalConstControl userData) + VTKM_CONT_EXPORT ArrayHandle(const PortalConstControl& userData) : Internals(new InternalStruct) { this->Internals->UserPortal = userData; diff --git a/vtkm/cont/internal/ArrayHandleExecutionManager.h b/vtkm/cont/internal/ArrayHandleExecutionManager.h index c0c3babab..01528facd 100644 --- a/vtkm/cont/internal/ArrayHandleExecutionManager.h +++ b/vtkm/cont/internal/ArrayHandleExecutionManager.h @@ -73,7 +73,7 @@ public: /// arrays, then this method may save the iterators to be returned in the \c /// GetPortalConst methods. /// - virtual void LoadDataForInput(PortalConstControl portal) = 0; + virtual void LoadDataForInput(cosnt PortalConstControl& portal) = 0; /// Allocates a large enough array in the execution environment and copies /// the given data to that array. The allocated array can later be accessed @@ -212,7 +212,7 @@ public: } VTKM_CONT_EXPORT - void LoadDataForInput(PortalConstControl portal) + void LoadDataForInput(const PortalConstControl& portal) { this->Transfer.LoadDataForInput(portal); } diff --git a/vtkm/cont/internal/ArrayManagerExecution.h b/vtkm/cont/internal/ArrayManagerExecution.h index f5e14ec8d..c10a2e881 100644 --- a/vtkm/cont/internal/ArrayManagerExecution.h +++ b/vtkm/cont/internal/ArrayManagerExecution.h @@ -83,7 +83,7 @@ public: /// GetPortalConst method. /// VTKM_CONT_EXPORT void LoadDataForInput( - typename ContainerType::PortalConstType portal); + const typename ContainerType::PortalConstType& portal); /// Allocates a large enough array in the execution environment and copies /// the given data to that array. The allocated array can later be accessed diff --git a/vtkm/cont/internal/ArrayManagerExecutionShareWithControl.h b/vtkm/cont/internal/ArrayManagerExecutionShareWithControl.h index 4adb54566..d231d59a7 100644 --- a/vtkm/cont/internal/ArrayManagerExecutionShareWithControl.h +++ b/vtkm/cont/internal/ArrayManagerExecutionShareWithControl.h @@ -64,7 +64,7 @@ public: /// Saves the given iterators to be returned later. /// - VTKM_CONT_EXPORT void LoadDataForInput(PortalConstType portal) + VTKM_CONT_EXPORT void LoadDataForInput(const PortalConstType& portal) { this->ConstPortal = portal; this->ConstPortalValid = true; diff --git a/vtkm/cont/internal/ArrayPortalFromIterators.h b/vtkm/cont/internal/ArrayPortalFromIterators.h index 17c311e6b..ea1f6770c 100644 --- a/vtkm/cont/internal/ArrayPortalFromIterators.h +++ b/vtkm/cont/internal/ArrayPortalFromIterators.h @@ -73,7 +73,7 @@ public: } VTKM_CONT_EXPORT - void Set(vtkm::Id index, ValueType value) const + void Set(vtkm::Id index, const ValueType& value) const { *this->IteratorAt(index) = value; } diff --git a/vtkm/cont/internal/ArrayTransfer.h b/vtkm/cont/internal/ArrayTransfer.h index cf7f59c55..c38d9db79 100644 --- a/vtkm/cont/internal/ArrayTransfer.h +++ b/vtkm/cont/internal/ArrayTransfer.h @@ -82,7 +82,7 @@ public: /// arrays, then this method may save the iterators to be returned in the \c /// GetPortalConst methods. /// - VTKM_CONT_EXPORT void LoadDataForInput(PortalConstControl portal) + VTKM_CONT_EXPORT void LoadDataForInput(const PortalConstControl& portal) { this->ArrayManager.LoadDataForInput(portal); } diff --git a/vtkm/cont/testing/UnitTestArrayContainerControlBasic.cxx b/vtkm/cont/testing/UnitTestArrayContainerControlBasic.cxx index d607181c3..dd4213d5c 100644 --- a/vtkm/cont/testing/UnitTestArrayContainerControlBasic.cxx +++ b/vtkm/cont/testing/UnitTestArrayContainerControlBasic.cxx @@ -48,7 +48,7 @@ struct TemplatedTests } } - bool CheckContainer(ArrayContainerType &array, ValueType value) + bool CheckContainer(ArrayContainerType &array, const ValueType& value) { for (IteratorType iter = array.GetPortal().GetIteratorBegin(); iter != array.GetPortal().GetIteratorEnd(); From ec2032e1d3088d23388d55374d09651632b77849 Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Mon, 19 May 2014 13:20:02 -0400 Subject: [PATCH 2/5] Correc the rest of the alignmnet issues. --- vtkm/cont/internal/ArrayHandleExecutionManager.h | 2 +- vtkm/cont/internal/ArrayPortalShrink.h | 2 +- vtkm/cont/internal/IteratorFromArrayPortal.h | 2 +- .../UnitTestArrayManagerExecutionShareWithControl.cxx | 6 +++--- vtkm/cont/testing/UnitTestArrayContainerControlBasic.cxx | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/vtkm/cont/internal/ArrayHandleExecutionManager.h b/vtkm/cont/internal/ArrayHandleExecutionManager.h index 01528facd..cc6e25248 100644 --- a/vtkm/cont/internal/ArrayHandleExecutionManager.h +++ b/vtkm/cont/internal/ArrayHandleExecutionManager.h @@ -73,7 +73,7 @@ public: /// arrays, then this method may save the iterators to be returned in the \c /// GetPortalConst methods. /// - virtual void LoadDataForInput(cosnt PortalConstControl& portal) = 0; + virtual void LoadDataForInput(const PortalConstControl& portal) = 0; /// Allocates a large enough array in the execution environment and copies /// the given data to that array. The allocated array can later be accessed diff --git a/vtkm/cont/internal/ArrayPortalShrink.h b/vtkm/cont/internal/ArrayPortalShrink.h index 5484e3af1..16d109b57 100644 --- a/vtkm/cont/internal/ArrayPortalShrink.h +++ b/vtkm/cont/internal/ArrayPortalShrink.h @@ -77,7 +77,7 @@ public: } VTKM_CONT_EXPORT - void Set(vtkm::Id index, ValueType value) const + void Set(vtkm::Id index, const ValueType& value) const { VTKM_ASSERT_CONT(index >= 0); VTKM_ASSERT_CONT(index < this->GetNumberOfValues()); diff --git a/vtkm/cont/internal/IteratorFromArrayPortal.h b/vtkm/cont/internal/IteratorFromArrayPortal.h index f8610a946..8dca771b5 100644 --- a/vtkm/cont/internal/IteratorFromArrayPortal.h +++ b/vtkm/cont/internal/IteratorFromArrayPortal.h @@ -60,7 +60,7 @@ struct IteratorFromArrayPortalValue } VTKM_CONT_EXPORT - ValueType operator=(ValueType value) + ValueType operator=(const ValueType& value) { this->Portal.Set(this->Index, value); return value; diff --git a/vtkm/cont/internal/testing/UnitTestArrayManagerExecutionShareWithControl.cxx b/vtkm/cont/internal/testing/UnitTestArrayManagerExecutionShareWithControl.cxx index a3f5ed1ad..101b29b51 100644 --- a/vtkm/cont/internal/testing/UnitTestArrayManagerExecutionShareWithControl.cxx +++ b/vtkm/cont/internal/testing/UnitTestArrayManagerExecutionShareWithControl.cxx @@ -40,7 +40,7 @@ struct TemplatedTests typedef vtkm::cont::internal::ArrayContainerControl< T, vtkm::cont::ArrayContainerControlTagBasic> ArrayContainerType; - void SetContainer(ArrayContainerType &array, ValueType value) + void SetContainer(ArrayContainerType &array, const ValueType& value) { std::fill(array.GetPortal().GetIteratorBegin(), array.GetPortal().GetIteratorEnd(), @@ -48,7 +48,7 @@ struct TemplatedTests } template - bool CheckArray(IteratorType begin, IteratorType end, ValueType value) + bool CheckArray(IteratorType begin, IteratorType end, const ValueType& value) { for (IteratorType iter = begin; iter != end; iter++) { @@ -57,7 +57,7 @@ struct TemplatedTests return true; } - bool CheckContainer(ArrayContainerType &array, ValueType value) + bool CheckContainer(ArrayContainerType &array, const ValueType& value) { return CheckArray(array.GetPortalConst().GetIteratorBegin(), array.GetPortalConst().GetIteratorEnd(), diff --git a/vtkm/cont/testing/UnitTestArrayContainerControlBasic.cxx b/vtkm/cont/testing/UnitTestArrayContainerControlBasic.cxx index dd4213d5c..c213072a7 100644 --- a/vtkm/cont/testing/UnitTestArrayContainerControlBasic.cxx +++ b/vtkm/cont/testing/UnitTestArrayContainerControlBasic.cxx @@ -38,7 +38,7 @@ struct TemplatedTests typedef typename ArrayContainerType::PortalType PortalType; typedef typename PortalType::IteratorType IteratorType; - void SetContainer(ArrayContainerType &array, ValueType value) + void SetContainer(ArrayContainerType &array, const ValueType& value) { for (IteratorType iter = array.GetPortal().GetIteratorBegin(); iter != array.GetPortal().GetIteratorEnd(); From 1ef967a426a6aa25d63c623b371900aef04406af Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Mon, 19 May 2014 14:20:59 -0400 Subject: [PATCH 3/5] Correct the option parser on windows. You can't include headers inside namespaces. --- vtkm/testing/OptionParser.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/vtkm/testing/OptionParser.h b/vtkm/testing/OptionParser.h index b6f221f97..c7c871b40 100644 --- a/vtkm/testing/OptionParser.h +++ b/vtkm/testing/OptionParser.h @@ -211,8 +211,12 @@ * */ -#ifndef OPTIONPARSER_H_ -#define OPTIONPARSER_H_ +#ifdef _MSC_VER +#include +#endif + +#ifndef vtk_m_testing_OPTIONPARSER_H_ +#define vtk_m_testing_OPTIONPARSER_H_ namespace vtkm { namespace testing { From 552a8ab16054824ed5707728374df5b98290bc4f Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Mon, 19 May 2014 14:23:30 -0400 Subject: [PATCH 4/5] Work around windows.h clobbering GetMessage. windows.h uses a macro to forward GetMessage to GetMessageA or GetMessageW, so to work around that we do the same on windows. --- vtkm/cont/Error.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/vtkm/cont/Error.h b/vtkm/cont/Error.h index b9d322da6..ed6c7be58 100644 --- a/vtkm/cont/Error.h +++ b/vtkm/cont/Error.h @@ -36,6 +36,14 @@ class Error public: const std::string &GetMessage() const { return this->Message; } +//GetMessage is a macro defined by to redirrect to +//GetMessageA or W depending on if you are using ansi or unicode. +//To get around this we make our own A/W variants on windows. +#ifdef _WIN32 + const std::string &GetMessageA() const { return this->Message; } + const std::string &GetMessageW() const { return this->Message; } +#endif + protected: Error() { } Error(const std::string message) : Message(message) { } From 763949351f8aec10896f7edf62839f261a25e2fd Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Mon, 19 May 2014 14:27:37 -0400 Subject: [PATCH 5/5] Stop trying to align vtkm types. Read the full comments on why. Using alignment on basic types when vtkm only targetted Linux/BSD/OSX was 'okay' because of how the alignment operators worked, but potential was going to cause issues in the long run if we failed to detect the correct size and the compiler was than forced to not use intrinsics. Now with adding windows support we have run into another problem. Basically using an alignment operator on a typedef means that the type must never be passed by value, but must always be passed by reference. The reason for this is that passing by value doesn't respect alignment requirements, and can cause very subtle errors or crashes. A really good read for people more interested in these problems: http://eigen.tuxfamily.org/dox/group__TopicPassingByValue.html http://eigen.tuxfamily.org/dox-devel/group__DenseMatrixManipulation__Alignement.html --- vtkm/Types.h | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/vtkm/Types.h b/vtkm/Types.h index d5f2049cb..b4fede606 100644 --- a/vtkm/Types.h +++ b/vtkm/Types.h @@ -107,16 +107,6 @@ typedef unsigned long long UInt64Type; #error Could not find a 64-bit integer. #endif -//for windows support we need a macro to wrap the alignment call, -//since for windows you need declspec(align) -#ifdef _MSC_VER -# define VTKM_ALIGN_BEGIN(size) __declspec(align(size)) -# define VTKM_ALIGN_END(size) -#else //we presume clang or gcc -# define VTKM_ALIGN_BEGIN(size) -# define VTKM_ALIGN_END(size) __attribute__((aligned(size))) -#endif - //----------------------------------------------------------------------------- template @@ -441,13 +431,11 @@ protected: }; /// Vector2 corresponds to a 2-tuple -namespace internal - { typedef VTKM_ALIGN_BEGIN(VTKM_ALIGNMENT_TWO_SCALAR) vtkm::Tuple Vector2 VTKM_ALIGN_END(VTKM_ALIGNMENT_TWO_SCALAR); } -typedef internal::Vector2 Vector2; +typedef vtkm::Tuple Vector2; /// Id2 corresponds to a 2-dimensional index -typedef VTKM_ALIGN_BEGIN(VTKM_SIZE_ID) vtkm::Tuple Id2 VTKM_ALIGN_END(VTKM_SIZE_ID); +typedef vtkm::Tuple Id2; template class Tuple @@ -522,7 +510,11 @@ protected: }; /// Vector3 corresponds to a 3-tuple -typedef VTKM_ALIGN_BEGIN(VTKM_SIZE_SCALAR) vtkm::Tuple Vector3 VTKM_ALIGN_END(VTKM_SIZE_SCALAR); +typedef vtkm::Tuple Vector3; + +/// Id3 corresponds to a 3-dimensional index for 3d arrays. Note that +/// the precision of each index may be less than vtkm::Id. +typedef vtkm::Tuple Id3; template class Tuple @@ -602,13 +594,9 @@ protected: }; /// Vector4 corresponds to a 4-tuple -typedef VTKM_ALIGN_BEGIN(VTKM_ALIGNMENT_FOUR_SCALAR) vtkm::Tuple Vector4 VTKM_ALIGN_END(VTKM_ALIGNMENT_FOUR_SCALAR); +typedef vtkm::Tuple Vector4; -/// Id3 corresponds to a 3-dimensional index for 3d arrays. Note that -/// the precision of each index may be less than vtkm::Id. -typedef VTKM_ALIGN_BEGIN(VTKM_SIZE_ID) vtkm::Tuple Id3 VTKM_ALIGN_END(VTKM_SIZE_ID); - /// Initializes and returns a Vector2. VTKM_EXEC_CONT_EXPORT vtkm::Vector2 make_Vector2(vtkm::Scalar x, vtkm::Scalar y)