-
Notifications
You must be signed in to change notification settings - Fork 16
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
Support for compiling on M1/M2 Macs #22
Comments
Hi @rob-p, I've just created a new branch here https://github.com/jermp/sshash/tree/apple-m-1-2-support with a possible fix. PS. We are doing the same for LPHash (https://github.com/jermp/lphash/blob/main/CMakeLists.txt#L17) and it works. |
Hi @jermp, This does seem to compile fine on my M1 MacBook Pro! I've not had a chance to test running it yet thought but will report back here once I get to that. --Rob |
Excellent @rob-p, thanks! |
Hi @jermp, So it starts to work, but then I get an exception.
This is with the unitigs from chr16 (computed using cuttlefish) as the input. Not sure why this might happen, but happy to dig deeper if you have any ideas. |
Mmmh that's strange. Can you try to see if PTHash (https://github.com/jermp/pthash) works correctly on M1? That exception is related to hash collisions....so another thing to try is to use 128-bit hash codes and see if the behavior persists: https://github.com/jermp/sshash/blob/apple-m-1-2-support/include/util.hpp#L26. |
Hi @rob-p, ok I've tested PTHash on M1 and everything works fine as expected. Compiling SSHash with Instead, if we compile with The problem is only on M1 with |
I've investigated some more and, actually, the problem seems the parallel mode of PTHash on M1. With 1 thread all is good. This means that the SSHash code is correct, no collisions during minimizers' calculation. Can you confirm this by using 1 thread here https://github.com/jermp/sshash/blob/apple-m-1-2-support/include/minimizers.hpp#L17 and here https://github.com/jermp/sshash/blob/apple-m-1-2-support/include/builder/build_skew_index.hpp#L136 on your chr16 example? Thanks Rob! |
sure! will try and report back. |
Confirmed. If I use 1 thread in both of these places, I can build the index successfully. |
See also this commit d603d25. |
Cool! Any idea why it otherwise breaks on ARM? |
I had one more thought /question. Right now, we can’t distribute sshash-based code via bioconda that will run on M1/2 machines. This is because bioconda is a bit weird. Currently, they don’t have any native M1/2 build hosts, so everything is compiled as x86_64. Usually, that’s OK (not optimal, but OK), because you can just install packages in an x86_64 environment and run them under rosetta 2. The problem is, that doesn’t work with this code. We immediately get an illegal instruction error. You can see this, e.g. if you pull down the latest One question I had is, what compiler flags do we actually need? For example, rosetta 2 handles many intrinsics fine (e.g. SSE4.2), but not others (e.g. AVX512). I am not certain if it handles BMI2 or not. My thought is, if we figured out what instructions it can handle — we could perhaps add a “CONDA_BUILD” flag to the CMake scripts that would skip Thanks! |
Fixed! jermp/pthash@348fe1d Already integrated on LPHash. I'm doing the same for SSHash and supersede d603d25. |
Ok, done. See the branch https://github.com/jermp/sshash/tree/apple-m-1-2-support. All good on my end. |
Oh, by the way, I'm about to also support larger k, up to 63. (Right now, under dev.) I think it will become increasingly useful to support k > 31. The space for SSHash will also become way smaller since, for fixed m, the number of super-k-mers will decrease. Happy to know what you think! |
Yeah, we should get an idea of what instructions are not permitted there...but anyway I do not think a x86 binary would execute well on ARM. Ok, we are mixing two separate (although related) things in this thread:
For point 2. I do not know since I haven't used bioconda and I'm not familiar with that. But if they do not have any M1/M2 build hosts...then it's not SSHash's (hence, nor our) fault :D Eventually they should get some building environment, I suppose, no? |
Hi @jermp, I agree, these are 2 separate issues. I'm opening the Bioconda one in another issue (since it's not really the same as this). Any thoughts on why the "normal" parallelization doesn't work under M1/2 though? Thanks! |
Hi @rob-p , it works now :) See comments above! |
Support for ARM has been added. I'm closing this @rob-p but feel free to reopen it. |
Hi @jermp,
Right now, compilation fails on M1 and M2 Macs for several reasons. First, the current version of Xcode doesn't support
-march=native
on these machines. Clang does upstream, so this will go away eventually, but it would be good for the CMakeFiles using this to have a flag one could pass to remove this flag.Second, there are several places where x86 intrinsics are used (and at least one place where inline ASM is used). The intrinsics could be made portable with simde, and the assembly could likely be tested too (not sure if that actually causes a problem or not but I can't get to it yet because of the other intrinsics).
I think it would be worth the (hopefully) minor changes to be able to support sshash (and hence piscem) compilation directly on M1/M2 hardware!
Best,
Rob
The text was updated successfully, but these errors were encountered: