Re: [eigen] two technical points: WithAlignedOperatorNew and std::complex casting

["Gael Guennebaud" <gael.guennebaud@xxxxxxxxx>]

On Wed, Dec 31, 2008 at 1:54 AM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
> 2008/12/25 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
>> Hi,
>>
>> On Wed, Dec 24, 2008 at 7:13 AM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>>
>>> 1) Currently, WithAlignedOperatorNew is empty if EIGEN_VECTORIZE is
>>> not defined. Which amounts to say, "if there's no vectorization, then
>>> no need to get aligned pointers". I think we should change that to
>>> letting WithAlignedOperatorNew be always the same i.e. always get
>>> aligned pointers. Reasons:
>>>
>>> a) the current solution seems dangerous to me if the user links
>>> together two files a.cpp and b.cpp using Eigen, if a.cpp is compiled
>>> without vectorization and b.cpp is compiled with vectorization, if a
>>> structure created in a.cpp is passed to b.cpp then b.cpp will expect
>>> it to be aligned...
>>> Notice that for static alignment (EIGEN_ALIGN_128) we already go for
>>> always-align-even-when-vectorization-is-disabled for similar reasons.
>>
>> ok, good point. So clearly we must be consistent here, and enable or
>> disable both EIGEN_ALIGN_128 and WithAlignedOperatorNew and not only
>> one or the other. The drawback to always align, even when this is
>> useless, is a possible waste of memory. On the other hand, I guess
>> that all architectures targeted by Eigen feature a vector unit
>> (SSE/Altivec), and so the vectorized path is aimed to be enabled 99.9%
>> of the time... and so I agree with your proposal to always enable
>> WithAlignedOperatorNew. One more argument: in the future, if we
>> realize this was not the right choice we can still go from "always
>> align" to "align if vectorization is enabled" without any trouble
>> while the other way round could break binary compatibility.
>
> Commited in r903399.
> The waste-of-memory issue is not a sufficient reason not to do that:
> 1) it is necessary anyway when vectorization is turned on
> 2) the binary compatibility issue prompting this change is a higher
> priority, when vectorization is not turned on
> Rather, we should educate our users about why doing
> struct foo : Eigen:WithAlignedOperatorNew{
>  Vector4f v;
>  float x;
> };
> foo *array = new foo[many];
> will result in a significant waste of memory (3/8 = 37.5% of occupied
> memory is alignment whitespace).

Perhaps I forgot to mention that in my mind, it is perfectly fine that
a lib/app compiled with vectorization don't have to be compatible with
a lib compiled with vectorization. It is just like many other GCC
options which break binary compatibility if both binaries are not
compiled with the same options.

In spite of what I said, eventually I'm not against to enforce
alignment everywhere but your example motivate me to add a "NoAlign"
or "Compact" flag for the fourth template parameter of Matrix<> such
that it is possible to use eigen to create compact structure at the
cost of no vectorization. Usage example:

typedef Matrix<float,4,1,NoAlign> Vector4Nf;
typedef Matrix<float,4,4,RowMajor|NoAlign> RowMatrix4Nf;

Of course this complexify a bit the use of Eigen, but this seems to be
a very important feature to me (an example among many others: to
create structures compatible with nvidia CUDA compiler)

cheers,
Gael.

---