From b7fc8746e07556404672b4da2d53dd323c441634 Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Fri, 27 Feb 2015 10:01:59 -0500 Subject: [PATCH 1/4] Replace a static assert by a runtime one, fixes the build of unit tests on ARM Also safely assert in the non-implemented path that should never be taken in practice, and would return wrong results. --- Eigen/src/Core/GenericPacketMath.h | 7 +++++-- Eigen/src/Core/util/StaticAssert.h | 3 +-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Eigen/src/Core/GenericPacketMath.h b/Eigen/src/Core/GenericPacketMath.h index 967a07df5..afdd6f71e 100644 --- a/Eigen/src/Core/GenericPacketMath.h +++ b/Eigen/src/Core/GenericPacketMath.h @@ -290,7 +290,11 @@ template EIGEN_DEVICE_FUNC inline Packet preverse(const Packet& template struct protate_impl { - static Packet run(const Packet& a) { return a; } + static Packet run(const Packet& a) + { + eigen_assert(false && "unimplemented"); + return a; + } }; /** \internal \returns a packet with the coefficients rotated to the right in little-endian convention, @@ -299,7 +303,6 @@ struct protate_impl */ template EIGEN_DEVICE_FUNC inline Packet protate(const Packet& a) { - EIGEN_STATIC_ASSERT(offset < unpacket_traits::size, ROTATION_BY_ILLEGAL_OFFSET); return offset ? protate_impl::run(a) : a; } diff --git a/Eigen/src/Core/util/StaticAssert.h b/Eigen/src/Core/util/StaticAssert.h index 5e16b775b..7538a0633 100644 --- a/Eigen/src/Core/util/StaticAssert.h +++ b/Eigen/src/Core/util/StaticAssert.h @@ -93,8 +93,7 @@ THE_STORAGE_ORDER_OF_BOTH_SIDES_MUST_MATCH, OBJECT_ALLOCATED_ON_STACK_IS_TOO_BIG, IMPLICIT_CONVERSION_TO_SCALAR_IS_FOR_INNER_PRODUCT_ONLY, - STORAGE_LAYOUT_DOES_NOT_MATCH, - ROTATION_BY_ILLEGAL_OFFSET + STORAGE_LAYOUT_DOES_NOT_MATCH }; }; From f5ff4d826fa5728f6319dcc7159b1337a2461880 Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Fri, 27 Feb 2015 10:56:50 -0500 Subject: [PATCH 2/4] Fix NEON build flags: in the current NDK, at least with the clang-3.5 toolchain, -mfpu=neon is not enough to activate NEON, since it's incompatible with the default float ABI, and I have to pass -mfloat-abi=softfp (which is what everyone does in practice). In fact, it would be a good idea to pass -mfloat-abi=softfp all the time, regardless of NEON. Also removing the -mcpu=cortex-a8, as 1) it's not needed and 2) if we really wanted to pass a specific -mcpu flag, that would presumably to tune performance for benchmarks, and it would then not really make sense to tune for the very old cortex-a8 (it reflects ARM CPUs from 5 years ago). --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 13f9c8f9d..2c1ae428e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -227,7 +227,7 @@ if(NOT MSVC) option(EIGEN_TEST_NEON "Enable/Disable Neon in tests/examples" OFF) if(EIGEN_TEST_NEON) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -mfpu=neon -mcpu=cortex-a8") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -mfpu=neon -mfloat-abi=softfp") message(STATUS "Enabling NEON in tests/examples") endif() From 33669348c490372e598f839754257f3710195337 Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Fri, 27 Feb 2015 11:35:37 -0500 Subject: [PATCH 3/4] Disable Packet2f/2i halfpacket support in NEON. I believe that it was erroneously turned on, since Packet2f/2i intrinsics are unimplemented, and code trying to use halfpackets just fails to compile on NEON, as it tries to use the default implementation of pload/pstore and the types don't match. --- Eigen/src/Core/arch/NEON/PacketMath.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Eigen/src/Core/arch/NEON/PacketMath.h b/Eigen/src/Core/arch/NEON/PacketMath.h index e9af45f22..12198d79c 100644 --- a/Eigen/src/Core/arch/NEON/PacketMath.h +++ b/Eigen/src/Core/arch/NEON/PacketMath.h @@ -76,12 +76,12 @@ typedef uint32x4_t Packet4ui; template<> struct packet_traits : default_packet_traits { typedef Packet4f type; - typedef Packet2f half; + typedef Packet4f half; // Packet2f intrinsics not implemented yet enum { Vectorizable = 1, AlignedOnScalar = 1, size = 4, - HasHalfPacket=1, + HasHalfPacket=0, // Packet2f intrinsics not implemented yet HasDiv = 1, // FIXME check the Has* @@ -95,12 +95,12 @@ template<> struct packet_traits : default_packet_traits template<> struct packet_traits : default_packet_traits { typedef Packet4i type; - typedef Packet2i half; + typedef Packet4i half; // Packet2i intrinsics not implemented yet enum { Vectorizable = 1, AlignedOnScalar = 1, size=4, - HasHalfPacket=1 + HasHalfPacket=0, // Packet2i intrinsics not implemented yet // FIXME check the Has* }; }; From 2fc3b484d71486bad6a43488a182fa7945d65f45 Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Fri, 27 Feb 2015 11:37:45 -0500 Subject: [PATCH 4/4] remove trailing comma --- Eigen/src/Core/arch/NEON/PacketMath.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Eigen/src/Core/arch/NEON/PacketMath.h b/Eigen/src/Core/arch/NEON/PacketMath.h index 12198d79c..8dd1e1370 100644 --- a/Eigen/src/Core/arch/NEON/PacketMath.h +++ b/Eigen/src/Core/arch/NEON/PacketMath.h @@ -100,7 +100,7 @@ template<> struct packet_traits : default_packet_traits Vectorizable = 1, AlignedOnScalar = 1, size=4, - HasHalfPacket=0, // Packet2i intrinsics not implemented yet + HasHalfPacket=0 // Packet2i intrinsics not implemented yet // FIXME check the Has* }; };