Split SSE cmake option. Use more descriptive names.

[j-rivero]

Current FCL_HAVE_SSE is enabling -march=native when compiled with gcc/clang. This flag enables much more than just SSE and it is specially dangerous when building packages.

Debian users were the first detecting this problem. Note that the error appeared again the ROS community, which uses a custom version of the package.

The current commit deprecate the option and change it by other two different ones:

• FCL_USE_HOST_CUSTOM_CFLAGS (default off): same effect than then previous one, hopefully with a more decriptive name.

• FCL_USE_X64_SSE (default on): enable the standard set of SEE flags supported in every x64 machine
  .
  I have included the set of cflags that we use to build the gazebo amd64 debian package. So far I did not get any bug report about them.

[jslee02]

This looks good to me. I have two suggestions that might be useful:

• Visual Studio x64 might complain about /arch:SSE2 as it's enabled by default. It would be nice to set the option only when the arch is x86. As I know, the arch name can be detected using CMAKE_VS_PLATFORM_NAME. For example,

  if(NOT "$CMAKE_VS_PLATFORM_NAME}" STREQUAL "Win64")
    add_definitions(/arch:SSE2)
  endif()

  I haven't tried with CMAKE_VS_PLATFORM_NAME so it should be tested for sure.

• It would be good to convey the compiler options to the consumer libraries using target_compile_options(). If I'm correct, the consumer library should be built with the same SIMD instructions. Otherwise, it causes segfaults.

Edit: Regarding the option name, FCL_USE_HOST_CUSTOM_CFLAGS sounds a bit too general to me while it adds flags only related to SIMD instructions. How about FCL_USE_ALL_AVAILABLE_SIMD_INSTRUCTIONS?

[jslee02]

DEPRECATION is not supported by CMake 2.8.12, which is the current minimum required CMake version of FCL. If I'm correct, FCL supports that version in order to support trusty.

In CMake 2.8.12, DEPRECATION will not cause any problem, but the message will be printed as DEPRECATIONFCL_USE_SSE is deprecated ... (no space after DEPRECATION). I would suggest we just use WARNING instead or use DEPRECATION only when it's supported.

[jmirabel]

My question might be a little too naive but is there a specific need justifying those cmake flags now ?

Before using Eigen, the flag FCL_USE_SSE was necessary because it was used to configure a preprocessor variable used in some #if ... #else ... #endif.

Now, all this is done by a combination of tools from the compiler and from Eigen. This automatic configuration is fine for most users. For advanced usage and/or optimization, one can always reconfigure via cmake -DCMAKE_CXX_FLAGS=foo.
I may be missing something here and would be grateful to be enlightened.

About the name of the flags, I suggest not to rename GCC options. Something like FCL_USE_HOST_NATIVE_ARCH or FCL_USE_NATIVE_ARCH for -march=native seems clearer to me.
The meaning of FCL_USE_X64_SSE is clear.

[j-rivero]

Visual Studio x64 might complain about /arch:SSE2 as it's enabled by default. It would be nice to set the option only when the arch is x86. As I know, the arch name can be detected using CMAKE_VS_PLATFORM_NAME. For example,

if(NOT "$CMAKE_VS_PLATFORM_NAME}" STREQUAL "Win64")
  add_definitions(/arch:SSE2)
endif()

I haven't tried with CMAKE_VS_PLATFORM_NAME so it should be tested for sure.

Looking into cmake code there seems to be more options than just Win32 for example itanium or x64. I found that the function implement values for CMAKE_VS_PLATFORM_NAME sets by default Win32 so I finally use it.

It would be good to convey the compiler options to the consumer libraries using target_compile_options(). If I'm correct, the consumer library should be built with the same SIMD instructions. Otherwise, it causes segfaults.

Added.

Edit: Regarding the option name, FCL_USE_HOST_CUSTOM_CFLAGS sounds a bit too general to me while it adds flags only related to SIMD instructions. How about FCL_USE_ALL_AVAILABLE_SIMD_INSTRUCTIONS?

I prefer your suggestion or @jmirabel's (FCL_USE_HOST_NATIVE_ARCH) than mine but don't have an strong preference on them. Feel free to propose what you think fits bests.

My question might be a little too naive but is there a specific need justifying those cmake flags now ?

That is a valid point. My personal view: if the machine has SSE support, go for it unconditionally. For the -march=native flag, I'm pretty sure that we want to control this since it is particularly bad for use cases like packaging but could have the potential of enabling very interesting features for compilations from source for users with powerful machines.

[davetcoleman]

I have confirmed these changes work for MoveIt! on Ubuntu 16.04 / ROS Kinetic in this travis build: https://travis-ci.org/davetcoleman/moveit_kinetic_cpp11

[jslee02]

I prefer your suggestion or @jmirabel's (FCL_USE_HOST_NATIVE_ARCH) than mine but don't have an strong preference on them. Feel free to propose what you think fits bests.

FCL_USE_HOST_NATIVE_ARCH sounds good to me as well. I think we can merge this once it's renamed to FCL_USE_HOST_NATIVE_ARCH.