Re: [eigen] Re: 3.0.4 coming soon, testing appreciated

[Jitse Niesen <jitse@xxxxxxxxxxxxxxxxx>]

On Sat, 26 Nov 2011, Benoit Jacob wrote:

> 2011/11/26 Jitse Niesen <jitse@xxxxxxxxxxxxxxxxx>:
> > [...] the assertion on MapBase.h:174 which is
> >
> >      eigen_assert(EIGEN_IMPLIES(internal::traits<Derived>::Flags&AlignedBit,
> > (size_t(m_data) % (sizeof(Scalar)*internal::packet_traits<Scalar>::size)) == 0)
> >        && "data is not aligned");
> >
> > AlignedBit is set and sizeof(Scalar) is 32, but m_data is only aligned on a
> > 16-byte boundary, so it bombs out.
>
> Clearly this assertion is wrong. It predates my changes (actually my
> changes were fixing the same mistake elsewhere). We never align to
> more than 16 bytes, so requiring higher alignment than that is wrong.
> The fix is to replace this by the same check that is performed in
> compute_matrix_flags.

Hmm, isn't the purpose of the assert to check that the pointer m_data is 
indeed aligned if it is claimed to be?

I'm quite happy to fix it, but I'm not sure what kind of alignment we 
expect. Is it as follows?

  If AlignedBit is set, then the address should be a multiple of
  the packet size (in bytes), unless a packet is bigger than 16 bytes,
  in which case the address should be a multiple of 16.

Or, to translate it to C++, the assertion in MapBase should be

   eigen_assert(EIGEN_IMPLIES(internal::traits<Derived>::Flags&AlignedBit,
     (size_t(m_data)
        % (std::min(sizeof(Scalar)*internal::packet_traits<Scalar>::size,
                    16))) == 0)

It probably still makes sense to put this in a template helper.


Jitse