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

Some htscodecs objects will soon need some target-specific compiler flags #1112

Closed
mp15 opened this issue Jun 20, 2022 · 4 comments
Closed

Comments

@mp15
Copy link

mp15 commented Jun 20, 2022

The current version of htslib uses a version of htscodecs which uses autoconf, automake, and libtool. devtools/import.py does not copy enough of the supporting files across, resulting in a spurious error when building on certain systems:

error: inlining failed in call to ‘always_inline’ ‘_mm256_insertf128_si256’: target specific option mismatch

This error is caused by the default makefile fall through (and thus the march has not been set correctly for this target).

@jmarshall
Copy link
Member

jmarshall commented Jun 20, 2022

There is no problem in current pysam, which uses the current HTSlib release — version 1.15.1, which does not include any files needing this treatment. However when we come to import a future release of HTSlib that does include this stuff, the pysam build will fail. Presumably this is what you are doing — trying to import current develop from HTSlib.

Ugh, who thought it would be a good idea to use automake or libtool? 😛 🤣

But that's a red herring for us anyway: pysam builds htscodecs using the bundled build rules in HTSlib's Makefile and does not use htscodecs's own build infrastructure. When pysam is built to use the bundled HTSlib, it always runs HTSlib's configure which does on these certain systems add the target flags in question to its generated config.mk:

$ python3 setup.py build
# pysam: cython is available - using cythonize if necessary
# pysam: htslib mode is shared
[…]
checking whether C compiler accepts -mssse3 -mpopcnt -msse4.1... yes
checking whether C compiler accepts -mavx2... yes
checking whether C compiler accepts -mavx512f... yes
checking whether C compiler supports ARM Neon... no

$ tail htslib/config.mk
[…]
# Extra CFLAGS for specific files
HTS_CFLAGS_AVX2 = -mavx2
HTS_CFLAGS_AVX512 = -mavx512f
HTS_CFLAGS_SSE4 = -mssse3 -mpopcnt -msse4.1
HTS_HAVE_NEON = 

However pysam does not use config.mk or the Makefile's rules directly to build these object files. Instead it queries LIBHTS_OBJS via make print-config and uses Python machinery to have each listed object file built using Python-appropriate flags.

Hence fixing this in due course will require:

  1. Probably having these HTS_CFLAGS_* variables printed out via HTSlib's make print-config so that they can be easily communicated to setup.py.
  2. Figuring out how to add some target-specific options to particular object files when using the Python machinery.

@jmarshall
Copy link
Member

Pysam imports are typically done from htslib/samtools/bcftools release tarballs. HTSlib release tarballs omit htscodecs's own autoconf/automake/libtool build infrastructure entirely, instead using HTSlib's own instructure to build a bundled htscodecs (or link against a previously-built external htscodecs). Pysam follows that, but doesn't do anything in particular to enable using an external htscodecs (it might be workable; I haven't tested it).

So in fact there's no additional supporting files that import.py ought to be copying across. The one candidate would be hts_probe_cc.sh, but as pysam always uses htslib/configure it's not vital to include it. (It would be easy to import it, though harder to avoid also unintentionally bringing test/**/*.sh along with it.)

@jmarshall jmarshall changed the title htscodecs insufficiently copied by devtools/import.py Some htscodecs objects will soon need some target-specific compiler flags Jun 20, 2022
@jkbonfield
Copy link

Ugh, who thought it would be a good idea to use automake or libtool? 😛 🤣

If you mean htscodecs, that would be me.

Libtool is a right pain and has a lot to hate about it, however it does make building shared libraries across multiple platforms an easy task. The alternative is a plethora of host specific mechanisms. Htslib attempts this for some systems, while other tools (eg Tcl) go one step further and essentially rewrite the host detection system so they can build almost anywhere. My hint when developing with a libtool system, is to use ./configure --disable-shared while debugging things to avoid most of the obscenities it adds!

Automake is a good thing IMO. The syntax is a bit ugh, but it adds so many features that people like and have asked for. Out of tree builds. "make dist", "make distcheck", etc. Features that htslib hasn't had for years or in some cases have rejected PRs to add. I find distcheck invaluable for spotting silly mistakes.

Hence fixing this in due course will require:

  1. Probably having these HTS_CFLAGS_* variables printed out via HTSlib's make print-config so that they can be easily communicated to setup.py.
  2. Figuring out how to add some target-specific options to particular object files when using the Python machinery.

Is there a reason you cannot simply cd to the htslib subdirectory, do ./configure && make, and then link that with pysam? You mention "python appropriate flags". What are they? Can they be specified on the configure line in e.g. CPPFLAGS instead? It feels like long term this would be a more maintainable system. Clearly pysam isn't rewriting htslib or grubbing around in the internals as it does have the ability to link against a system htslib, so it must work OK with the vanilla set of exports and standard include files.

@jmarshall
Copy link
Member

Is there a reason you cannot simply cd to the htslib subdirectory, do ./configure && make, and then link that with pysam?

Just that this is always the way it has been done, and it is easiest to use the Python infrastructure that is being used to build Cython code to build this too. However it looks to be pretty difficult or impossible to specify compilation flags for individual translation units.

So with PR #1140 now merged, pysam has moved to building libhts.a using HTSlib's build infrastructure instead. It would be even tidier if it took advantage of HTSlib's separate build directory capabilities, and I will likely finish up that draft at a later date.

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

No branches or pull requests

3 participants