Match default integer sizes to standard interfaces

[insertinterestingnamehere]

This matches the default integers to size_t and ptrdiff_t so that they match the size of a pointer even on LLP64 platforms like Windows.
I also went ahead and changed the default f77_int definition from long to int to more closely match the Fortran and C blas interfaces. Though, to my knowledge, it's not standardized anywhere, Fortran compilers usually use 32 bit integers by default.

I'm in the process of testing this myself. Putting this here for discussion for now.

[njsmith]

These two changes both match my understanding of the de facto standards here too.

[tkelman]

I think

blis/frame/include/bli_config_macro_defs.h

Line 158 in 2dc5c0a

#define BLIS_BLAS2BLIS_INT_TYPE_SIZE 64

may also need to change to 32 to be consistent with this. Essentially every binary distribution of any open-source BLAS implementation that I've ever seen has used 32 bit integers, with the exception of a new, separate, symbols-renamed openblas64 package in Fedora (OpenMathLib/OpenBLAS#646). MKL is the exception here and generally won't play well with other applications unless you recompile everything against MKL with matching integer sizes.

[jeffhammond]

Github refused to accept my comment via the web interface so I will try again via email... Lots of Fortran applications compile with `-i8` or its equivalent, meaning that they cause the default `INTEGER` to be 64b. The Fortran interface needs to be flexible and allow the user to build with either 32b or 64b integers.

[njsmith]

@jeffhammond: Sure, the argument isn't that BLIS should only support 32-bits, but rather that the BLIS defaults (and the "public API", i.e. the API used in widely-distributed shared library builds) should match the fortran defaults.

[jeffhammond]

@tkelman No, that's just not true. MKL plays just fine with apps as long as users link against the correct library - LP64 or ILP64.

$ ls -1 /opt/intel/mkl/lib/*lp*
/opt/intel/mkl/lib/libmkl_blas95_ilp64.a
/opt/intel/mkl/lib/libmkl_blas95_lp64.a
/opt/intel/mkl/lib/libmkl_intel_ilp64.a
/opt/intel/mkl/lib/libmkl_intel_ilp64.dylib
/opt/intel/mkl/lib/libmkl_intel_lp64.a
/opt/intel/mkl/lib/libmkl_intel_lp64.dylib
/opt/intel/mkl/lib/libmkl_lapack95_ilp64.a
/opt/intel/mkl/lib/libmkl_lapack95_lp64.a

[tkelman]

@jeffhammond not if your application (edit: distributed as a binary) tries to load an LP64 libblas.dylib simultaneously. Many many mex files have caused matlab to segfault for this reason. dgemm_ should have exactly one integer size. Fortran linking convenience in the "compile everything from source" HPC environment has driven design decisions that cause countless segfaults in "millions of users doing technical computing on laptops" environments.

[jeffhammond]

@njsmith Sorry, but there is no truly default Fortran integer size. Fortran does not have an ABI like C does. There is a default integer type (INTEGER), whose size is set by the compiler. Compilers tend to default to 32b, but it is really stupid for assume this, because many codes set it to 64b and this decision will break all of those.

The real solution is for Fortran users to call C symbols with Fortran 2003 ISO_C_BINDING, but obviously people who are still using software that's older than I am (Netlib BLAS) can't be troubled to learn about such things.

The ideal solution would be for BLIS to support the standard cblas interface and both 32b and 64b Fortran binary libraries, but this is probably real work for somebody.

[tkelman]

The ideal solution would be for BLIS to support the standard cblas interface and both 32b and 64b Fortran binary libraries, but this is probably real work for somebody.

We're essentially proposing doing that work here, just have to start with selecting a default. The argument is that for 32 bit and 64 bit Fortran binary interfaces to coexist in a world where libraries get distributed as binaries, one of them needs a different name.

[jeffhammond]

@tkelman dgemm_ should have exactly one integer size just like Mac should have a case-sensitive filesystem and no one should abandon their pet. Good luck with that.

Matlab is capable of calling MKL properly. If users are writing broken MEX code, that's their problem.

It sucks that Fortran isn't like C, e.g. does not have an ABI. You should consider WG5 to change this. Until then, please try not to break user experience for thousands of quantum chemists who have arrays with more than a billion elements in them.

[jeffhammond]

@tkelman Just give them both a descriptive name. Put 32 in one and 64 in the other. Why do you have to pick a default and remove the bitwidth from that one?

[njsmith]

Just give them both a descriptive name. Put 32 in one and 64 in the other. Why do you have to pick a default and remove the bitwidth from that one?

If we could go back and time this would be a great solution but... making dgemm_ default to 32-bit would break x% of existing code, and require x% of users to override the defaults. Making dgemm_ default to 64-bit would break y% of existing code, and require y% of users to override the defaults. Not providing dgemm_ at all (your new proposal) would break 100% of existing code, and require 100% of users to rewrite their code. x is less than y is less than 100, and changing defaults is easier than rewriting code, so we should default to 32-bits and allow users to override that if they want.

Note that e.g. Debian and Ubuntu have a standard ABI for all the BLAS packages they ship, which allows users to pick which BLAS they want to use at runtime. This ABI uses symbols named like dgemm_ and 32-bit integers. If BLIS doesn't default to this, then Debian and Ubuntu will just patch their version to force it to comply...

[insertinterestingnamehere]

So, the primary reason I'm suggesting the change to f77_int is because it matches the netlib cblas and f77 blas headers. The default expectation for BLAS integers is already established there. A BLAS with better chosen indices and a 64 bit compatible interface would be great, but it's better to have something that conforms to the already standard ABI.

[tkelman]

The user experience is probably already broken if those quantum chemists' applications happen to load numpy into the same address space. Maybe every HPC center has a custom ILP64 build of numpy or only ever uses MKL, but I'll defer to @njsmith on whether he's come across that kind of situation (or whether ILP64 MKL is likely to work with numpy at all? I believe the commercial numpy distributions all use LP64 BLAS).

[njsmith]

@tkelman: well, because python doesn't use RTLD_GLOBAL we are less prone to symbol clashes than Julia, but the underlying issues still exist... I'm not sure whether numpy supports 64-bit BLAS at all (no reason it couldn't, but such libraries are extremely rare in the wild, so it may never have been wedged into the giant nest of #ifdef's required to handle the cases that do actually in the wild). And e.g. SciPy actually re-exports the raw BLAS/LAPACK ABI to other packages just because portably linking to BLAS/LAPACK is such a pain (in fact I think this was implemented by @insertinterestingnamehere? :-)), and the ABI it expects to export is the 32-bit one.

[njsmith]

@insertinterestingnamehere: @tkelman is right though that your patch needs to also change the default value of BLIS_BLAS2BLIS_INT_TYPE_SIZE, not just the fallback behavior when it isn't defined (because in practice it always gets defined to a default value :-)).

[tkelman]

If users are writing broken MEX code, that's their problem

Well, it's a library maintainer's problem for anyone writing a library that calls BLAS and might be used in an environment where ILP64 BLAS happens to be loaded in the same process. You get libraries that work fine outside of Matlab / pre-symbol-renaming Julia / -i8 HPC application but segfault within them.

because python doesn't use RTLD_GLOBAL we are less prone to symbol clashes than Julia

I'm actually not so sure whether Julia is using RTLD_GLOBAL automatically in our default ccall setup. The code ends up really easy to use but this is one of the trickier pieces of Julia's code base (for me) to understand due to the LLVM interaction. We do expose dlopen to users though and there are cases in the wild where a RTLD_GLOBAL ends up being the easiest way to get C libraries to be able to call into Julia's own copy of BLAS/LAPACK, e.g. https://github.com/JuliaOpt/SCS.jl/blob/0a28aadaffeea8f06e8e0924a000d6a8d8ce0dab/src/SCS.jl#L15-L16.

[jeffhammond]

@njsmith You're right, dgemm has to be there and I guess 32b has to be the default, since I don't want @mbanck and friends to have to patch it downstream in Debian.

However, it would be useful to have a build without dgemm since I could write my own interface to dgemm32 and dgemm64 that works in modern Fortran codes.

@tkelman Major quantum chemistry packages do not use Numpy - they use Fortran :-) I suspect that a quantum chemist who used Numpy would be so productive that they would not have to bother with the Fortran packages with the legacy cruft that motivated my comments in the first place.

[insertinterestingnamehere]

@njsmith Yes, I wrote and now maintain the Cython API in SciPy. Identifying the correct BLAS library, dealing with the different name mangling conventions, and patching any bugs (see scipy/scipy#4105) are all massive barriers for someone who is trying to build a small-ish Python extension. That makes platform independent code hard to find and hard to write. For distribulting modules it is vastly easier for everyone to load the BLAS and LAPACK routines they need via capsules in Cython's cimport mechanisms.

[insertinterestingnamehere]

I've updated this to fix the two places where BLIS_BLAS2BLIS_INT_TYPE_SIZE may have been set to 64. AFAICT, that's all that is needed.

[njsmith]

This is discussing the BLAS layer so I'm pretty sure you mean int, not size_t?

[insertinterestingnamehere]

Thanks, yes. I'll make that change.

[insertinterestingnamehere]

Updated.

[insertinterestingnamehere]

Sorry, it looks like a bunch of my edits to the Windows build system got rolled in to this. I'll pull that out as well.

[njsmith]

@insertintetestingnamehere: did you mean to include a bunch of py3 changes to the windows build dir here? (NB also that it's not clear the windows build dir is useful -- for Windows build we've just been using the regular build system with a mingw cross-compiler, and if #13 is fixed then we should be able to use the regular build system with msys2 for native builds.)

[insertinterestingnamehere]

@njsmith no, that was the result of habitual use of git commit -a. I've force pushed yet again to get a clean version up.

[njsmith]

I guess you could use intptr_t and uintptr_t here, and remove the need for stddef.h? Otherwise looks good to me.

[insertinterestingnamehere]

I opted to use size_t and ptrdiff_t because they accurately describe the fact that these pointers will be used to address things within a single object. Unfortunately, there are some architectures weird enough that size_t is not always the same as uintptr_t (http://stackoverflow.com/a/14068191/1935144). There's also the fact that MSVC 2008 doesn't have a stdint.h by default, so, if/when we have a build system capable of using msvc 2008 for the reference kernels, we'd be stuck providing platform specific definitions for these integers to provide sensible default behavior.
It seemed simpler to rely on stddef.h instead.

[njsmith]

I don't think the stdint.h dependency is a problem, given that BLIS uses stdint.h typedefs pervasively (e.g., 2 lines above this one). And I'm not sure that anyone will ever care about MSVC 2008 anyway -- for those who need MSVC 2008 ABI compatibility, there's mingwpy, and mingwpy isn't restricted to the reference kernels. As for segmented architectures, I guess anyone trying to ported BLIS to one of those should specify a BLIS_INT_TYPE_SIZE instead of hoping that this fallback code will somehow do something intelligent :-).

But sure, this is fundamentally just a matter of taste and doesn't matter too much. Maybe @fgvanzee will clarify which way he prefers :-).

[insertinterestingnamehere]

Yes, it's certainly a matter of preference. Whichever version the maintainers prefer is fine with me.

[devinamatthews]

At this point in time, the default BLAS integer size is 32-bit and the default internal integer size is 64-bit if defined(_M_X64) || defined(__x86_64) || defined(__aarch64__) || defined(_ARCH_PPC64) and 32-bit otherwise. This seems to cover all of the concerns of this PR (except for separate problems outlined in #43). Unless anyone objects I think this PR should be closed.

NB: The third branch in the #if <32-bit> #elif <64-bit> #else ... #endif constructs should never happen, as these values are checked in the configure script. If someone edits the config.h file directly they get what they deserve.