-
Notifications
You must be signed in to change notification settings - Fork 370
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
Enable shared library build on Travis CI #37
Conversation
@tkelman I assume that answers your question? That said, there may still be an issue on bulldozer systems, since Travis gives us no guarantee of the architecture on which the tests will be run. (Last I checked, the systems were haswell.) |
My issue may have been caused by building for a series of configurations in a row, something like:
(though also doing a cross compile with some extra flags, so the problem may be coming from there too) |
Okay this might just be a case of my cross-compiler not knowing what to do with bulldozer-specific FMA instructions?
|
I'm getting the same kind of errors, but building with MIngw-w64 compilers under Cywgin, and for sandybridge (but not reference):
This is an old Opteron machine. |
Are your Cygwin packages up to date ( |
Yes, I just installed Cywin this evening:
My make flags are the same as yours:
|
Curiously, I can reproduce the same error with a mingw-w64 v5.3.1 gcc, but not with a native linux v5.3.1 gcc. |
Are you configuring from in tree or a separate build dir? |
Using a separate build dir in both cases. |
Actually now on Ubuntu 14.04 I am seeing the bulldozer linker errors from just
|
Does it error on a build that isn't parallel? |
It does, that was the next thing I tried |
to fix linking issue mentioned in flame#37 and https://groups.google.com/forum/#!topic/blis-devel/iypwljcaeEI
Found a fix for the bulldozer issue, see PR #40. Meanwhile, adding testing of the shared-library build configuration to Travis is a good thing, right? |
@tkelman: Regarding adding the testing of the shared library build to Travis, I think we should only do that if the shared library is built properly (ie: we expect it to work well), and to be honest, I'm not an expert in properly building a shard library. I used to think it was a trivial variation on static archives, but after some conversations with @Maratyszcza, I'm less sure. Perhaps Marat can chime in here. |
okay fine maybe this is a bit on the complicated side after all |
@tkelman Symbol interposition makes everything more complicated. |
The main thing is that you want to make sure that only the symbols that you intend to make public actually are public. (I guess this is what Marat means about interposition?) The generally recommended best practice is to compile with The other thing to worry about is abi stability and versioning. That said, I'm not sure why it would be bad to start testing that the shared libraries function technically, even if they're not yet ready for public consumption. For example, this would make it easier to tell that future modifications like an |
It's not entirely clear to me either what is meant by "symbol interposition". That said, making the default visibility hidden and then explicitly exporting the public API seems like a great approach to me. |
Symbol interposition means that publicly exported symbols can be overriden by symbols in previously loaded dynamic libraries or the executable, i.e. if your internal names are not too unique, horrible things may happen. I agree that |
I think one attractive approach would be to start by marking the BLAS/CBLAS symbols as public in shared-library builds, and hiding everything else. That way we could get started on the shared library infrastructure as one piece of work, and then over time expand the exported API incrementally as and when we're able to review the newly public functions and make a commitment that they can be supported over a longer time period in a shared-library stable-ABI style. (Of course the full API would still be available from static BLIS builds, which don't need to worry about ABI stability in the same way.) |
I don't see why you would want to hide BLIS' full API. There isn't that much code written against it relative to classic Fortran BLAS or CBLAS APIs yet, but hiding it makes that code harder to write and use in environments where shared libraries are preferred. From experience, the Fortran and CBLAS API's definitely need some mechanism of renaming the symbols if you're using ILP64 interfaces with shared libraries if you ever expect to have other code that you did not personally compile loaded into the same address space. It's relatively easy to do this renaming if you start from a static library and a list of symbols, but on OSX it requires a non-standard GPL-licensed tool (http://www.agner.org/optimize/#objconv) that isn't part of binutils, so this should maybe not be BLIS's job to handle immediately. |
For LLP64 platforms, it'd be ideal to use |
Because I don't know if Field is ready to commit to maintaining ABI compatibility on the full API, or even necessarily which parts of the API are even intended to be public. And in fact I would like for him to break ABI compatibility :-) Right now all the strided array functions the BLIS API measure strides in units of
Right, the standard BLAS/CBLAS |
If we want to start worrying about API and/or ABI stability then throw on an SONAME for now, but don't hesitate to keep changing things as long as it gets annotated as such. Most of these stability issues apply equally to linking against a static BLIS library, so this is as much a general documentation issue about what's public and/or stable vs what isn't. If it ever becomes a non-theoretical concern that say Julia and NumPy might both want to have incompatible shared-library versions of BLIS loaded simultaneously (and what consequences that would have w.r.t. embedding one language in the other), then we can consider getting really fancy with symbol versioning if we really need to. |
@tkelman: yeah, bumping the SONAME every time anything changes is also an option. But this does still require that someone notice when an ABI-breaking change is made (which in turn probably means that we really do want to export only the actually-intended-for-public-consumption functions, not every single internal function as well). And I'm not so much worried about NumPy and Julia clashing, as e.g. Debian getting cranky and refusing to ship BLIS if the SONAME changes every 2 weeks. |
The likelihood of that right now is why Debian shipping BLIS is still very much theoretical too. |
@tkelman: It's not that theoretical -- one of NumPy's Debian maintainers was playing with this last week. And I think Debian would have no problem at all with shipping a version of libblis.so that just exported the BLAS/CBLAS symbols, possibly alongside a libblis-dev package that contained the |
Fair enough. But if I or anyone else wanted to write a Julia package BLIS.jl (I actually started in this direction several years ago, but there's not much there of interest right now) that leverages the full BLIS API, I'll need a shared library. A Debian package with a libblis.so built without the full API (publicly visible) would be useless for such a Julia package, I'd have to check for symbols and skip loading that package and force a compilation from source, making the Julia package take considerably longer to install. You could argue that maybe the extended BLIS API should perhaps live in a separate shared library than the [C]BLAS compatibility layer, so the Debian maintainer could set up |
Ah, fair enough. But if the BLIS API is not stable, then a Debian package is also useless, isn't it, because you'll want to pin the particular version of BLIS that your wrapper is targeting?
Unfortunately, I think this is impossible given ELF's symbol namespace model: if Okay, so here's what I'd suggest as an incremental path forward for enabling shared-BLIS:
|
Only because Debian maintainers have a habit of removing old versions of things much sooner than downstream projects are actually ready to use new API-breaking versions, rather than making old and new versions simultaneously installable. If there are frequent API breakages, debian packaging does more harm than help. If an API is useful, and designed to be utilized by downstream projects, then exporting it by default has value - otherwise it should just be |
Details: - After merging PR #303, at Isuru's request, I removed the use of BLIS_EXPORT_BLIS from all function prototypes *except* those that we potentially wish to be exported in shared/dynamic libraries. In other words, I removed the use of BLIS_EXPORT_BLIS from all prototypes of functions that can be considered private or for internal use only. This is likely the last big modification along the path towards implementing the functionality spelled out in issue #248. Thanks again to Isuru Fernando for his initial efforts of sprinkling the export macros throughout BLIS, which made removing them where necessary relatively painless. Also, I'd like to thank Tony Kelman, Nathaniel Smith, Ian Henriksen, Marat Dukhan, and Matthew Brett for participating in the initial discussion in issue #37 that was later summarized and restated in issue #248. - CREDITS file update.
Details: - After merging PR flame#303, at Isuru's request, I removed the use of BLIS_EXPORT_BLIS from all function prototypes *except* those that we potentially wish to be exported in shared/dynamic libraries. In other words, I removed the use of BLIS_EXPORT_BLIS from all prototypes of functions that can be considered private or for internal use only. This is likely the last big modification along the path towards implementing the functionality spelled out in issue flame#248. Thanks again to Isuru Fernando for his initial efforts of sprinkling the export macros throughout BLIS, which made removing them where necessary relatively painless. Also, I'd like to thank Tony Kelman, Nathaniel Smith, Ian Henriksen, Marat Dukhan, and Matthew Brett for participating in the initial discussion in issue flame#37 that was later summarized and restated in issue flame#248. - CREDITS file update.
Details: - After merging PR flame#303, at Isuru's request, I removed the use of BLIS_EXPORT_BLIS from all function prototypes *except* those that we potentially wish to be exported in shared/dynamic libraries. In other words, I removed the use of BLIS_EXPORT_BLIS from all prototypes of functions that can be considered private or for internal use only. This is likely the last big modification along the path towards implementing the functionality spelled out in issue flame#248. Thanks again to Isuru Fernando for his initial efforts of sprinkling the export macros throughout BLIS, which made removing them where necessary relatively painless. Also, I'd like to thank Tony Kelman, Nathaniel Smith, Ian Henriksen, Marat Dukhan, and Matthew Brett for participating in the initial discussion in issue flame#37 that was later summarized and restated in issue flame#248. - CREDITS file update.
Details: - After merging PR flame#303, at Isuru's request, I removed the use of BLIS_EXPORT_BLIS from all function prototypes *except* those that we potentially wish to be exported in shared/dynamic libraries. In other words, I removed the use of BLIS_EXPORT_BLIS from all prototypes of functions that can be considered private or for internal use only. This is likely the last big modification along the path towards implementing the functionality spelled out in issue flame#248. Thanks again to Isuru Fernando for his initial efforts of sprinkling the export macros throughout BLIS, which made removing them where necessary relatively painless. Also, I'd like to thank Tony Kelman, Nathaniel Smith, Ian Henriksen, Marat Dukhan, and Matthew Brett for participating in the initial discussion in issue flame#37 that was later summarized and restated in issue flame#248. - CREDITS file update.
Seeing some issues locally with linking the bulldozer shared library, let's see if Travis reproduces it.