Alignment issues with Eigen datatypes and vectorization
Hi there, first of all, thanks for this easy-to-use and well documented library!
There however seem to be alignment issues with the Eigen datatypes when compiling the library and software including its headers, which can lead to segmentation faults with compiling with vectorization (SSE) enabled. This probably is the case as you use Eigen datatypes like Eigen::Vector2d as a member of a structs/classes or in stl containers, like here:
https://github.com/ethz-asl/grid_map/blob/master/grid_map_core/include/grid_map_core/Polygon.hpp#L206
See https://eigen.tuxfamily.org/dox-devel/group__DenseMatrixManipulation__Alignement.html for more information on the particular issues.
As far as I can see:
- best would be to adapt the code to use the Eigen allocators with correct alignment for vectorization
- second best to use
Eigen::DontAligndatatypes to prevent alignment issues - third best to at least warn the user to turn off Eigen vectorization as otherwise it can lead to hard-to-debug segmentation faults (see Eigen docs linked above for details on each of the three options)
Dear @mjschuster, thanks a lot! This might actually be an issue and I can look into this at a later point. If you have a suggestion on how to implement this fix, feel free to create a PR.
Dear @pfankhauser, thanks! It'd be great if you could have a look at this. I myself probably won't have the time for a pull request on this issue anytime soon. My suggestion would be to go through the code and see if its possible to
- use Eigen aligned allocators for all structs / classes having fixed-size vectorizable Eigen datatypes as members (see https://eigen.tuxfamily.org/dox-devel/group__TopicStructHavingEigenMembers.html) (that should be easy)
- make sure not to pass Eigen objects by value (see https://eigen.tuxfamily.org/dox-devel/group__TopicPassingByValue.html)
- use Eigen aligned allocators for all stl containers with fixed-size alignable Eigen datatypes or classes containing them (see https://eigen.tuxfamily.org/dox-devel/group__TopicStlContainers.html) (this might be the most difficult / annoying part)
As a workaround so far, I just deactivated vectorization by defining EIGEN_DONT_VECTORIZE and EIGEN_DISABLE_UNALIGNED_ARRAY_ASSERT as suggested in https://eigen.tuxfamily.org/dox-devel/group__TopicUnalignedArrayAssert.html to keep ABI compatibility.
I however suspect that typical operations performed on grid maps could greatly benefit from vectorization - it would be interesting to have a speed comparison. (As far as I understand it, this is only about explicit vectorization in the Eigen library. So a good compiler with appropriate optimization settings might still be able to add some vectorization through SSE etc. instructions on its own -- though probably less optimal?)
Hi all,
as we had some alignment related crashes with grid_map, so I added a small fix concerning the alignment issues with Eigen. Although I did not have time to thoroughly look through all the code, I added a missing EIGEN_MAKE_ALIGNED_OPERATOR_NEW in the GridMap class, which has fixed-size Eigen members.
I created a pull request with this fix. But I have to add, that as I did not check the entire code, there could be lurking other alignment issues.
Best regards
Thanks, @ThomasEmter! I merged your PR and extended it with other places where this code had fixed-size Eigen members. Let me know if we need to work more on this issue or you have any other problems.
Dear Péter!
Thank you very much for fixing the other places. We did not have any crashes anymore with my fix already.
Best regards, Thomas P.S.: And thank you for this great mapping tool ☺
Thanks @ThomasEmter @pfankhauser ! While these fixes should address some of the alignment issues, I think there still are some more (and more complicated) ones left, in particular with respect to my second and third bullet point in the comment above.
For example in https://github.com/ethz-asl/grid_map/blob/master/grid_map_core/include/grid_map_core/Polygon.hpp#L206 you create a std::vector of Position, which is a typedef to Eigen::Vector2d (see https://github.com/ethz-asl/grid_map/blob/master/grid_map_core/include/grid_map_core/TypeDefs.hpp#L18).
According to https://eigen.tuxfamily.org/dox-devel/group__TopicStlContainers.html you should use the Eigen allocators then: "In practice you must use the Eigen::aligned_allocator (not another aligned allocator), and #include <Eigen/StdVector>.", e.g. something like:
std::vector<Position, Eigen::aligned_allocator<Position> > vertices_;
In addition, according to https://eigen.tuxfamily.org/dox-devel/group__TopicPassingByValue.html these Eigen types should not be passed by value, as is done for example in https://github.com/ethz-asl/grid_map/blob/master/grid_map_core/include/grid_map_core/Polygon.hpp#L157 and many other places.
Thus, while it might work for now without crashes, there are several places left that can cause trouble in the future...
Hi all,
Thanks @mjschuster for pointing out other Eigen aligment issues.
I fixed all std::vector Eigen aligment issues I could find. I hope I have found all and I also did not find any other std container type in the code that would need fixing.
Hi all,
I did test the alignment fixes with the current master and rebased the fixes accordingly.