Re: [eigen] Link error: multiple definition of ei_p*** functions

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

Hi,

Here's a patch fixing the issue.

But it is not just fixing the issue, it is proposing a general solution.

The problem of how to define functions in our headers without causing
multiple definition linking errors, is one we've faced many times. We
have had basically two approaches to work around it:
 - either make the function have static linkage. Big drawback: this
means binary code bloat as the resulting executable may have N copies
of the code of that function, if you linked together N object files.
 - or mark the function as inline. This fixes the issue nicely, but
has the unwanted side effect of hinting the compiler toward inlining
the function. Fortunately it's just a hint, so in cases where that
would be a really bad idea, the compiler still won't inline it.

Since there is no ideal solution, my diff introduces two new macros:
#define EIGEN_DECLARE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS
#define EIGEN_DEFINE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS inline

see, the above values are for choosing the 'inline' approach, which I
prefer. If you prefer the 'static' approach, set the 1st one to
'static' and the 2nd one to empty. The point of such macros is 1) to
make our intent clear with these inline keywords, and 2) that if
eventually a better solution appears, we can easily switch to it (even
if it's not a drop-in replacement, these macros will be easy to search
for).

Opinions?

Benoit

2010/6/16 Manoj Rajagopalan <rmanoj@xxxxxxxxx>:
> Hi,
>
>   I just began to notice these link errors in the "release" version of my code which I compile with SSE2 - multiple definitions of the
> packet mathfunctions seem to be present in different .o files which is causing linker death. I just refreshed my working copy
> before the build. Would someone know why this is happening? Perhaps some "inline" keyword is missing?
>
>   My g++ command-line flags are as follows:
>
> -Wall -Werror -march=pentium4 -g0 -O3 -msse2 -mfpmath=sse
> -ftree-vectorize -fgcse-sm -fgcse-las -fgcse-after-reload -fipa-pta
> -ftree-loop-linear -ftree-loop-im -ftree-loop-ivcanon -ftracer -funroll-loops
> -fvariable-expansion-in-unroller -fprefetch-loop-arrays -fno-math-errno
> -ffinite-math-only -fno-trapping-math -fno-signaling-nans
> -DEIGEN_NO_AUTOMATIC_RESIZING
>
>
>   A linker error log snippet is:
>
> kmesh.o: In function `float __vector Eigen::ei_psqrt<float __vector>(float __vector const&)':
> kmesh.cpp:(.text+0x60): multiple definition of `float __vector Eigen::ei_psqrt<float __vector>(float __vector const&)'
> iv.o:iv.cpp:(.text+0x60): first defined here
> kmesh.o: In function `float __vector Eigen::ei_pcos<float __vector>(float __vector const&)':
> kmesh.cpp:(.text+0xae): multiple definition of `float __vector Eigen::ei_pcos<float __vector>(float __vector const&)'
> iv.o:iv.cpp:(.text+0x33ee): first defined here
> kmesh.o: In function `float __vector Eigen::ei_psin<float __vector>(float __vector const&)':
> kmesh.cpp:(.text+0x20e): multiple definition of `float __vector Eigen::ei_psin<float __vector>(float __vector const&)'
> iv.o:iv.cpp:(.text+0x354e): first defined here
> kmesh.o: In function `float __vector Eigen::ei_pexp<float __vector>(float __vector const&)':
> kmesh.cpp:(.text+0x38c): multiple definition of `float __vector Eigen::ei_pexp<float __vector>(float __vector const&)'
> iv.o:iv.cpp:(.text+0x36cc): first defined here
> kmesh.o: In function `float __vector Eigen::ei_plog<float __vector>(float __vector const&)':
> kmesh.cpp:(.text+0x4a2): multiple definition of `float __vector Eigen::ei_plog<float __vector>(float __vector const&)'
> iv.o:iv.cpp:(.text+0x37e2): first defined here
> offdiagfunctors.o: In function `float __vector Eigen::ei_psqrt<float __vector>(float __vector const&)':
> offdiagfunctors.cpp:(.text+0x3e0): multiple definition of `float __vector Eigen::ei_psqrt<float __vector>(float __vector const&)'
> iv.o:iv.cpp:(.text+0x60): first defined here
> offdiagfunctors.o: In function `float __vector Eigen::ei_plog<float __vector>(float __vector const&)':
> offdiagfunctors.cpp:(.text+0xb24): multiple definition of `float __vector Eigen::ei_plog<float __vector>(float __vector const&)'
> iv.o:iv.cpp:(.text+0x37e2): first defined here
>
>
> thanks,
> Manoj
>
>
>
>

diff --git a/Eigen/src/Core/GenericPacketMath.h b/Eigen/src/Core/GenericPacketMath.h
--- a/Eigen/src/Core/GenericPacketMath.h
+++ b/Eigen/src/Core/GenericPacketMath.h
@@ -205,29 +205,34 @@ template<typename Packet> inline typenam
 template<typename Packet> inline Packet ei_preverse(const Packet& a)
 { return a; }
 
 /**************************
 * Special math functions
 ***************************/
 
 /** \internal \returns the sin of \a a (coeff-wise) */
-template<typename Packet> inline static Packet ei_psin(const Packet& a) { return ei_sin(a); }
+template<typename Packet> EIGEN_DECLARE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS
+Packet ei_psin(const Packet& a) { return ei_sin(a); }
 
 /** \internal \returns the cos of \a a (coeff-wise) */
-template<typename Packet> inline static Packet ei_pcos(const Packet& a) { return ei_cos(a); }
+template<typename Packet> EIGEN_DECLARE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS
+Packet ei_pcos(const Packet& a) { return ei_cos(a); }
 
 /** \internal \returns the exp of \a a (coeff-wise) */
-template<typename Packet> inline static Packet ei_pexp(const Packet& a) { return ei_exp(a); }
+template<typename Packet> EIGEN_DECLARE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS
+Packet ei_pexp(const Packet& a) { return ei_exp(a); }
 
 /** \internal \returns the log of \a a (coeff-wise) */
-template<typename Packet> inline static Packet ei_plog(const Packet& a) { return ei_log(a); }
+template<typename Packet> EIGEN_DECLARE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS
+Packet ei_plog(const Packet& a) { return ei_log(a); }
 
 /** \internal \returns the square-root of \a a (coeff-wise) */
-template<typename Packet> inline static Packet ei_psqrt(const Packet& a) { return ei_sqrt(a); }
+template<typename Packet> EIGEN_DECLARE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS
+Packet ei_psqrt(const Packet& a) { return ei_sqrt(a); }
 
 /***************************************************************************
 * The following functions might not have to be overwritten for vectorized types
 ***************************************************************************/
 
 /** \internal \returns a * b + c (coeff-wise) */
 template<typename Packet> inline Packet
 ei_pmadd(const Packet&  a,
diff --git a/Eigen/src/Core/arch/SSE/MathFunctions.h b/Eigen/src/Core/arch/SSE/MathFunctions.h
--- a/Eigen/src/Core/arch/SSE/MathFunctions.h
+++ b/Eigen/src/Core/arch/SSE/MathFunctions.h
@@ -25,17 +25,17 @@
 
 /* The sin, cos, exp, and log functions of this file come from
  * Julien Pommier's sse math library: http://gruntthepeon.free.fr/ssemath/
  */
 
 #ifndef EIGEN_MATH_FUNCTIONS_SSE_H
 #define EIGEN_MATH_FUNCTIONS_SSE_H
 
-template<> EIGEN_DONT_INLINE EIGEN_UNUSED
+template<> EIGEN_DEFINE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS EIGEN_UNUSED
 Packet4f ei_plog<Packet4f>(const Packet4f& _x)
 {
   Packet4f x = _x;
   _EIGEN_DECLARE_CONST_Packet4f(1 , 1.0f);
   _EIGEN_DECLARE_CONST_Packet4f(half, 0.5f);
   _EIGEN_DECLARE_CONST_Packet4i(0x7f, 0x7f);
 
   _EIGEN_DECLARE_CONST_Packet4f_FROM_INT(inv_mant_mask, ~0x7f800000);
@@ -105,17 +105,17 @@ Packet4f ei_plog<Packet4f>(const Packet4
   y = ei_padd(y, y1);
   x = ei_psub(x, tmp);
   y2 = ei_pmul(e, ei_p4f_cephes_log_q2);
   x = ei_padd(x, y);
   x = ei_padd(x, y2);
   return _mm_or_ps(x, invalid_mask); // negative arg will be NAN
 }
 
-template<> EIGEN_DONT_INLINE EIGEN_UNUSED
+template<> EIGEN_DEFINE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS EIGEN_UNUSED
 Packet4f ei_pexp<Packet4f>(const Packet4f& _x)
 {
   Packet4f x = _x;
   _EIGEN_DECLARE_CONST_Packet4f(1 , 1.0f);
   _EIGEN_DECLARE_CONST_Packet4f(half, 0.5f);
   _EIGEN_DECLARE_CONST_Packet4i(0x7f, 0x7f);
 
 
@@ -180,17 +180,17 @@ Packet4f ei_pexp<Packet4f>(const Packet4
    take into account the special handling they have for greater values
    -- it does not return garbage for arguments over 8192, though, but
    the extra precision is missing).
 
    Note that it is such that sinf((float)M_PI) = 8.74e-8, which is the
    surprising but correct result.
 */
 
-template<> EIGEN_DONT_INLINE EIGEN_UNUSED
+template<> EIGEN_DEFINE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS EIGEN_UNUSED
 Packet4f ei_psin<Packet4f>(const Packet4f& _x)
 {
   Packet4f x = _x;
   _EIGEN_DECLARE_CONST_Packet4f(1 , 1.0f);
   _EIGEN_DECLARE_CONST_Packet4f(half, 0.5f);
 
   _EIGEN_DECLARE_CONST_Packet4i(1, 1);
   _EIGEN_DECLARE_CONST_Packet4i(not1, ~1);
@@ -281,17 +281,17 @@ Packet4f ei_psin<Packet4f>(const Packet4
   y2 = _mm_and_ps(poly_mask, y2);
   y = _mm_andnot_ps(poly_mask, y);
   y = _mm_or_ps(y,y2);
   /* update the sign */
   return _mm_xor_ps(y, sign_bit);
 }
 
 /* almost the same as ei_psin */
-template<> EIGEN_DONT_INLINE EIGEN_UNUSED
+template<> EIGEN_DEFINE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS EIGEN_UNUSED
 Packet4f ei_pcos<Packet4f>(const Packet4f& _x)
 {
   Packet4f x = _x;
   _EIGEN_DECLARE_CONST_Packet4f(1 , 1.0f);
   _EIGEN_DECLARE_CONST_Packet4f(half, 0.5f);
 
   _EIGEN_DECLARE_CONST_Packet4i(1, 1);
   _EIGEN_DECLARE_CONST_Packet4i(not1, ~1);
@@ -370,17 +370,17 @@ Packet4f ei_pcos<Packet4f>(const Packet4
   y  = _mm_or_ps(y,y2);
 
   /* update the sign */
   return _mm_xor_ps(y, sign_bit);
 }
 
 // This is Quake3's fast inverse square root.
 // For detail see here: http://www.beyond3d.com/content/articles/8/
-template<> EIGEN_UNUSED
+template<> EIGEN_DEFINE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS EIGEN_UNUSED
 Packet4f ei_psqrt<Packet4f>(const Packet4f& _x)
 {
 	Packet4f half = ei_pmul(_x, ei_pset1(.5f));
 	
 	/* select only the inverse sqrt of non-zero inputs */
 	Packet4f non_zero_mask = _mm_cmpgt_ps(_x, ei_pset1(std::numeric_limits<float>::epsilon()));
 	Packet4f x = _mm_and_ps(non_zero_mask, _mm_rsqrt_ps(_x));
 
diff --git a/Eigen/src/Core/util/Macros.h b/Eigen/src/Core/util/Macros.h
--- a/Eigen/src/Core/util/Macros.h
+++ b/Eigen/src/Core/util/Macros.h
@@ -157,16 +157,23 @@
 #if (defined __GNUC__)
 #define EIGEN_DONT_INLINE __attribute__((noinline))
 #elif (defined _MSC_VER)
 #define EIGEN_DONT_INLINE __declspec(noinline)
 #else
 #define EIGEN_DONT_INLINE
 #endif
 
+// this macro allows to get rid of linking errors about multiply defined functions.
+//  - static is not very good because it prevents definitions from different object files to be merged.
+//           So static causes the resulting linked executable to be bloated with multiple copies of the same function.
+//  - inline is not perfect either as it unwantedly hints the compiler toward inlining the function.
+#define EIGEN_DECLARE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS
+#define EIGEN_DEFINE_FUNCTION_ALLOWING_MULTIPLE_DEFINITIONS inline
+
 #if (defined __GNUC__)
 #define EIGEN_DEPRECATED __attribute__((deprecated))
 #elif (defined _MSC_VER)
 #define EIGEN_DEPRECATED __declspec(deprecated)
 #else
 #define EIGEN_DEPRECATED
 #endif