Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

building failed on Arm64 MacOS #1686

Closed
fastcoding opened this issue Dec 25, 2020 · 26 comments
Closed

building failed on Arm64 MacOS #1686

fastcoding opened this issue Dec 25, 2020 · 26 comments

Comments

@fastcoding
Copy link

Here are errors when I built it on a macbookpro with apple's new m1 processor:

In file included from /Volumes/Samsung_T5/opensource/libigl/include/igl/FastWindingNumberForSoups.h:374:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.0/include/emmintrin.h:13:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.0/include/xmmintrin.h:13:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.0/include/mmintrin.h:33:5: error: 
      use of undeclared identifier '__builtin_ia32_emms'; did you mean
      '__builtin_isless'?
    __builtin_ia32_emms();
...
@alecjacobson
Copy link
Contributor

For my own reference, I can appear to recreate this on my intel mac using:

clang++ -target arm64-apple-macos10.15 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk  -c -std=c++11 -DIGL_STATIC_LIBRARY ./include/igl/fast_winding_number.cpp -I ./include/ -I ./external/eigen

Seems to also be popping up online for other libraries. Hopefully can find a solution.

@jdumas
Copy link
Collaborator

jdumas commented May 29, 2021

There are solutions to translate SSE instructions to NEON, e.g. using https://github.com/DLTcollab/sse2neon or https://github.com/simd-everywhere/simde. But this would require adding additional dependencies to libigl, so imho we should move the FastWindingNumberForSoups code into a separate module and fix the upstream code accordingly.

@alecjacobson
Copy link
Contributor

Did the upstream code fix this SSE issue?

@jdumas
Copy link
Collaborator

jdumas commented May 29, 2021

No the upstream code is the same, but if we add the sse2neon dependency to the fast winding number code, I think we should do it in the upstream repository rather than keep it inside libigl.

@alecjacobson
Copy link
Contributor

It doesn't seem like Neil is continuing to maintain that code. If we start maintaining it, why keep it in a separate repository?

@jdumas
Copy link
Collaborator

jdumas commented May 29, 2021

We could maintain it as a fork, like we do for triangle or tetgen. But jamming everything into a single header file and embedding it in libigl has several disadvantages. First you removed the TBB dependency, which is a performance hit. Second, others may want to use the code directly without going pulling all of libigl, and I think it's valuable to have a self-contained piece of code for that. Third, making everything header-only has other drawbacks, as discussed in #1696 (header pollution, compilation times, cannot separate interface from implementation, etc.).

@stigersh
Copy link
Contributor

stigersh commented Jun 21, 2021

Hi,
I found this issue, seems to be my problem as well, I have a MacOs Big Sur with the M1 chip and I'm failing to compile libigl as a static library. Is FastWindingNumberForSoups the only thing going wrong? Is there any workarounds to work with M1 processor, as it seems by the comments as a design change which will take time to fix..
Thanks a lot for reading this anyway.

The errors after calling cmake and then make are just the same as posted here.

@svenpilz
Copy link
Contributor

svenpilz commented Jul 6, 2021

I have similar problems building for Emscripten (WebAssembly). Would it be possible to add a preprocessor flag to disable FastWindingNumberForSoups? Version v2.2.0 works fine.

In file included from include/igl/signed_distance.h:14:
In file included from include/igl/fast_winding_number.h:4:
In file included from include/igl/FastWindingNumberForSoups.h:374:
emsdk/upstream/emscripten/cache/sysroot/include/compat/emmintrin.h:11:2: error: "SSE2 instruction set not enabled"
#error "SSE2 instruction set not enabled"

Edit: Just for the record, even with activated SIMD support in Emscripten it will not compile, because _mm_setcsr is needed and not available in Emscripten's compatibility mode (https://emscripten.org/docs/porting/simd.html).

@jdumas
Copy link
Collaborator

jdumas commented Aug 28, 2021

FYI I have created a PR against the original WindingNumber code, adding support for M1 architecture via SIMDE:
sideeffects/WindingNumber#3

@Tibus
Copy link

Tibus commented Oct 31, 2021

Hi,
Any news on that?
Libigl is not compatible with ARM (Apple M1) due to this bug and don't know what to do.

Thanks a lot and sorry for my poor english :D

@alecjacobson
Copy link
Contributor

We're working on a non-vectorized replacement and should have it integrated soon. For now, if you don't need fast winding numbers, then use libigl in header only mode.

@Tibus
Copy link

Tibus commented Nov 1, 2021

I'm using Fast Winding Number for an Offset Iso-Surface feature.
I'm currently using the header-only mode but is there a way to make FastWindingNumber working on ARM without the header-only mode?
Otherwise I will need to wait until the non-vectorized FastWinding come up.
Thanks for your hard work on libigl! Really love it!

@jdumas
Copy link
Collaborator

jdumas commented Nov 1, 2021

If you read my message above, you will see that I have a fork of the FastWindingNumber repository that works on M1:
sideeffects/WindingNumber#3

This work is not integrated in libigl, but if you really want to use the fast winding numbers on M1, you should be able to do so via this fork.

@jpek42
Copy link

jpek42 commented Feb 11, 2022

@jdumas - Thanks for the PR to try to fix this!

Looking at the PR, it's hard to know how to go about integrating it into a project that uses libigl's fast winding number header. Can you clarify in the PR? You've added cmake stuff. Does that mean there are extra dependencies? We're using libigl in header-only mode.

Thanks!

@jdumas
Copy link
Collaborator

jdumas commented Feb 11, 2022

Yes there's an extra dependency which is SIMDe (this does the translation of intrinsics). To use it just put these lines of codes in your CMake project:

include(FetchContent)
FetchContent_Declare(
    WindingNumber
    GIT_REPOSITORY https://github.com/jdumas/WindingNumber.git
    GIT_TAG        cd81346bc8e31a5a77330b841d001e0efbf12f7f
)
FetchContent_MakeAvailable(WindingNumber)

target_link_libraries(my_lib PUBLIC WindingNumber::WindingNumber)

@jpek42
Copy link

jpek42 commented Feb 13, 2022

Thank you. Does this mean that it's not possible to use this in header-only mode?

@jdumas
Copy link
Collaborator

jdumas commented Feb 13, 2022

No.

@jpek42
Copy link

jpek42 commented Feb 13, 2022

Can you explain? If I'm reading this right, it's introducing a link-time dependency on the WindingNumber project?

@jdumas
Copy link
Collaborator

jdumas commented Feb 13, 2022

You have two options:

  • Use the winding number code embedded into libigl. Header-only, does not depend on TBB. Does not support macOS M1.
  • Use my fork of the upstream repository. Depends on TBB and SIMDe. Supports macOS M1. Not header-only.

@alecjacobson
Copy link
Contributor

With #1975, the status for M1 compilation is:

LIBIGL_EMBREE → not working (need to bump embree)
LIBIGL_MATLAB → not tested
LIBIGL_MOSEK → not tested

Everything else compiles and tests pass except tests/test_igl_copyleft_comiso. I've never used the comiso module and I wonder if this test was passing well on non-M1s.

@alecjacobson
Copy link
Contributor

#1976 fixes LIBIGL_EMBREE

Matlab doesn't yet support native m1 source and neither does mosek source.

One option for Matlab, would be to use apple's rosetta (build x86_64 binaries), this is a bit of a pain for dependencies though, in particular CGAL's dependences on gmp and mpfr, which are gnarly to build from source.

@alecjacobson
Copy link
Contributor

Meanwhile, it does seem possible to build libigl (including LIBIGL_RESTRICTED_MATLAB and LIBIGL_COPYLEFT_CGAL) using Rosetta. Here's a proof of concept script.

I'm not sure if it makes sense / is possible to cmake-ify this script into something that would be easy to expose as a cmake option.

@svenpilz
Copy link
Contributor

I have similar problems building for Emscripten (WebAssembly). Would it be possible to add a preprocessor flag to disable FastWindingNumberForSoups? Version v2.2.0 works fine.

FYI, 2.4.0 compiles fine again on Emscripten.

@alecjacobson
Copy link
Contributor

great to know! thanks

@alecjacobson
Copy link
Contributor

With #1985 merged, LIBIGL_COPYLEFT_CGAL compiles with rosetta. This means that libraries compiling with LIBIGL_RESTRICTED_MATLAB can compile under rosetta and link to the rosetta matlab on M1s.

So keeping tally, only

LIBIGL_MOSEK → untested

remains.

@alecjacobson
Copy link
Contributor

This seems to all be working now. I still haven't tried mosek but that modules also getting kind of out date anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants