dthuerck / mapmap_cpu

A high-performance general-purpose MRF MAP solver, heavily exploiting SIMD instructions.
BSD 3-Clause "New" or "Revised" License
103 stars 51 forks source link

FORCEINLINE macro is not clang compatible #5

Closed nmoehrle closed 6 years ago

nmoehrle commented 6 years ago

I think the FORCEINLINE macro is not clang compatible, while it appears to work with gcc I think the proper way to specify the always inline is __attribute__((always_inline)) instead of __always_inline. Further I think that it should be decided based on the compiler rather than the operating system which macro to use.

So I would suggest something like mentioned here:

#ifdef __MSVC__
#define FORCEDINLINE __forceinline
#else
#define FORCEDINLINE __attribute__((always_inline))
#endif

I think this the cause for nmoehrle/mvs-texturing#98.

dthuerck commented 6 years ago

Thanks for raising the issue!

Checking for the compiler in lieu of the OS sounds reasonable; it seems that for GCC, __attribute__((always_inline)) throws lots of warnings when not followed by inline. Thus, I added both.

With this and some smaller changes in 410b1f89d972646828724cba00fcd57a023acb82, I'm now able to build the package with Clang 3.8.0 if (and only if) link time optimization is disabled (remove -flto from the build parameters) -- otherwise, there's a link error to LLVMgold.so (Ubuntu 16.04).

Note: Clang 3.8 builds about 10% slower code than GCC 5.2 on my system with the default parameters in my CMakeLists.txt, link time optimizations disabled.

nmoehrle commented 6 years ago

Thanks for the quick response! :-) Hope that'll make the macOS guys happy ;-)