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

travis: added arm64, ppc64le and s390x tests with simde #283

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

junaruga
Copy link

@junaruga junaruga commented May 17, 2020

This PR is to add arm64, ppc64le and s390x tests with simde.

I referred Debian's Makefile and the patch to use simde, as there are Debian's bwa deb packages on some CPU architectures here.

For .travis.yml and how to implement simde, I referred minimap2.

This PR also fixes #77 and #71 .

Travis is ok here.

@@ -15,6 +15,11 @@ INCLUDES=
LIBS= -lm -lz -lpthread
SUBDIRS= .

ifneq ($(simde),)
CFLAGS += -DUSE_SIMDE -DSIMDE_ENABLE_NATIVE_ALIASES -DSIMDE_ENABLE_OPENMP -fopenmp-simd
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the explanation of -fopenmp-simd.
https://gcc.gnu.org/legacy-ml/gcc-patches/2013-10/msg02275.html

And here is the explanation of SIMDE_ENABLE_OPENMP in the simde README.md.

For best performance, in addition to -O3 (or whatever your compiler's equivalent is), you should enable OpenMP 4 SIMD support by defining SIMDE_ENABLE_OPENMP before including any SIMDe headers, and enabling OpenMP support in your compiler.

@martin-g
Copy link
Contributor

TravisCI has been replaced with Github Actions some time ago - 765fac1.
But still I think TravisCI should be revived to run the tests on the architectures which are not available at Github Actions!

This PR will also solve the two linked issues above!

Ping @jmarshall .

@jmarshall
Copy link
Contributor

jmarshall commented Apr 20, 2022

In my opinion, a better solution for mainline bwa than either proposed PR would be for someone who cares to implement native ARM NEON versions of ksw_u8() and ksw_i16(). [This has now been done; see PR #359.]

As for Travis: bwa's use of Travis CI was replaced because the service had become unreliable. I have seen no evidence of that having changed since; see also the conversation on lh3/minimap2#740.

But I am not a bwa maintainer (so don't ping me).

@martin-g
Copy link
Contributor

But I am not a bwa maintainer (so don't ping me).

Apologies! I pinged you because you did the migration from TravisCI to Github Actions.

@mr-c
Copy link

mr-c commented Apr 20, 2022

In my opinion, a better solution for mainline bwa than either proposed PR would be for someone who cares to implement native ARM NEON versions of ksw_u8() and ksw_i16().

Agreed! And one can still use SIMDe as a fallback for non-X86, non-ARM

@jmarshall
Copy link
Contributor

And one can still use SIMDe as a fallback for non-X86, non-ARM

SIMDe may be of interest to people such as Debian packagers who would like to have green lights across all their ports. Unless you can point to e.g. a community doing genomics on s390x, it may be of less interest to mainline bwa IMHO.

@junaruga
Copy link
Author

The s390x is a big-endian while x86_64, arm64, ppc64le are a little-endian. So, if someone wants to keep bwa with a big-endian friendly too, the s390x line might be good.

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

Successfully merging this pull request may close these issues.

Fails compile on POWER8: SSE2 intrinsics -> VSX/AltiVec
4 participants