BLD: Add Accelerate support for macOS 13.3+ by thalassemia · Pull Request #19816 · scipy/scipy

thalassemia · 2024-01-06T03:35:17Z

I added a script (scipy/_build_utils/_generate_blas_wrapper.py) to generate C wrappers for Accelerate BLAS/LAPACK. These wrappers are compiled into a static library that other modules can link against with no code modifications (e.g. a call to dgemm in a Fortran module will be routed through a wrapper and turned into a call to dgemm$NEWLAPACK). I also added Accelerate support to the script (scipy/linalg/_generate_pyx.py) that generates the Cython for _fblas/_flapack using similar C wrappers. In the process of making these two changes, I included special logic to handle the G77 ABI wrappers, eliminating the need for additional Fortran wrappers.

Other changes:

Functions in _fblas/_flapack that have a return value were written as such (previously, f2py generated Fortran wrappers for these)
- UPDATE: Reverting this change because the intermediate Fortran wrappers are needed to guarantee compatibility of complex return types, without which segfaults can occur on x86 machines (BLD: Revamp BLAS/LAPACK G77 ABI wrappers and fix PROPACK segfaults #19855)
- UPDATE 2: Brought this change back to avoid erroneous F2PY boilerplate that referenced incorrect BLAS/LAPACK symbols. Fortran subroutine wrappers were translated into C and appended to G77 ABI wrapper file (BLD: Refactor BLAS/LAPACK wrapper infrastructure #20232)
Resolved missing prototype build warnings in scipy/integrate/lsoda.pyf and scipy/integrate/vode.pyf
- UPDATE: Separated into BLD: resolve missing prototype warnings in lsoda/vode #19821
Added test for cython_blas.cladiv and cython_blas.dladiv
- UPDATE: Separated into TST: linalg: Test Cython LAPACK complex ladiv #19820
Declared separate blas_dep and lapack_dep with necessary compile arguments, leaving blas and lapack untouched for more useful information when calling scipy.show_config()
Resolved function with no return value build warnings and remove custom G77 wrappers in PROPACK (depends on separate PR on the PROPACK repo)
- UPDATE: Separated into BLD: Revamp BLAS/LAPACK G77 ABI wrappers and fix PROPACK segfaults #19855

Downsides

The wrapper functions are all in separate source files, allowing the linker to exclude unused symbols and keep binary sizes about the same as they currently are. Since there are ~1600 BLAS/LAPACK functions, this increases build time by a noticeable amount. I tried writing everything to a single file and compiling with -ffunction-sections -fdata-sections -Wl,-dead_strip, but this still resulted in unused symbols and larger binaries.

There is no way to mix and match BLAS/LAPACK libraries at the moment (also a challenge to support both LP64 and ILP64).

Future

This wrapper generation framework makes it pretty easy to add ILP64 support. If this PR seems OK in principle, I can make a separate PR detailing some quirks I encountered trying to implement ILP64 support with this as a base.

rgommers · 2024-04-12T19:04:45Z

Thanks @thalassemia. Working on going through this once more and exercising it a bit.

the build times seem to be under control as evidenced by my M1 Accelerate GHA taking in 10 minutes while the M1 OpenBLAS test took 11 minutes.

Agreed. I see a 10%-15% increase in build time on my M1 Macbook Pro. Which I agree is acceptable. Tracing shows that the added targets are adding a ~20 sec load of small file compiles at the start of the build, which makes sense:

The build time increase will only occur with Accelerate, so that's all good.

rgommers · 2024-04-13T06:56:36Z

I checked the binary size change: this approach is very lean, 54 kb extra due to the extra static libraries. Size grows from 21.78 MB to 21.84 MB for a local build. The macosx_arm64 wheels on PyPI are 30.3 MB, so this is (as expected) a major win - we can start shipping macosx_14_0_arm64 wheels that are almost a third smaller than the macosx_12_0_arm64 ones.

I could not figure out how to get the macOS linker to exclude unused symbols when consolidating all the wrappers into a single compilation target. I wonder if this might be possible on other platforms with other linkers (will explore in a future ILP64 PR).

I think it's possible with linker flags like -Wl,--gc-sections perhaps, but not in a portable way. Enabling LTO may help - see #16098 (comment).

scipy/spatial/meson.build

scipy/meson.build

thalassemia · 2024-04-14T01:06:44Z

Good news! I think I found a way to put every wrapper in a single file and let the linker exclude unused symbols. Previously, the linker was forced to include otherwise unused symbols simply because all exported symbols are global by default. Now, by explicitly marking my wrapper symbols as hidden, the linker can remove any symbols that are not directly used within each individual module.

Importantly, this PR only adds the flags needed to make this work in macOS, since that is the only platform with Accelerate. In a future ILP64 PR using this framework, I'll need to add additional compiler-specific Meson flags to achieve the same result (-ffunction-sections -fdata-sections -Wl,--gc-sections for GCC and /Gy for MSVC)

scipy/_build_utils/src/wrap_dummy_g77_abi.c

scipy/_build_utils/src/wrap_g77_abi.c

scipy/_build_utils/_generate_blas_wrapper.py

[skip circle] [skip cirrus]

scipy/_build_utils/src/npy_cblas.h

meson.build

rgommers

Previously, the linker was forced to include otherwise unused symbols simply because all exported symbols are global by default.

Regarding hidden symbols, it shouldn't be needed to define those manually or use __attribute__('hidden') directly I suspect. Instead, the gnu_symbol_visibility keyword to static_library can be used to do this automatically - and we wouldn't have to worry about things like Clang defining __GNU__. Did you consider this?

rgommers · 2024-04-14T08:20:16Z

scipy/sparse/linalg/_dsolve/meson.build

    'SuperLU/SRC/zutil.c'
  ],
  c_args: _superlu_lib_c_args,
+  dependencies: blas_dep,


That this worked at all before looks like a bug. It should have failed without this and USE_VENDOR_BLAS=1 set. The reason it doesn't is probably all the aliasing done in slu_Cnames.h.

Looking at this more closely, I think this line is unnecessary. Even if the BLAS dependency is missing when compiling the static library, it is included when compiling the extension module that links with this library. The same applies to the BLAS/LAPACK dependencies I added to the ARPACK static library. I'll clean those up for clarity.

scipy/special/meson.build

rgommers · 2024-04-14T19:54:23Z

Re gnu_symbol_visibility, I just realized that we should be using that anyway on all static libraries - see gh-20477.

thalassemia · 2024-04-14T20:22:08Z

Did not know about the gnu_symbol_visibility option. That is a lot cleaner and works for me locally using either OpenBLAS or Accelerate. I updated my PR to use that.

[skip circle] [skip cirrus]

rgommers

LGTM now. Time to give this a go. Thanks a lot @thalassemia! And thanks @h-vetinari for all the review effort!

h-vetinari · 2024-04-15T23:41:52Z

Congratulations @thalassemia and thanks for splitting various pieces off from this big effort! This definitely deserves a release note - if you like, please edit one into the current draft.

andyfaff · 2024-04-15T23:45:49Z

Yes, congratulations @thalassemia, a big effort! Do we know how much the wheel size will reduce with not having to bundle openblas?

thalassemia · 2024-04-16T00:38:08Z

@andyfaff On my machine (M2, macOS 14.4.1, Python 3.11), 34edaf2 on main yields a wheel 23.1MB in size. On the same machine with OpenBLAS 0.3.26, the current 1.13.0 release tag (7dcd8c5) yields a wheel 31.8MB in size. This 27% reduction is close to what was reported earlier in this PR before I found a way to efficiently combine all the wrappers into a single source file.

Do you think it makes sense to have our macOS wheels use Accelerate by default? At the moment, Accelerate is only used if users build from source with -Dblas=accelerate.

andyfaff · 2024-04-16T00:45:08Z

Do you think it makes sense to have our macOS wheels use Accelerate by default?

It's only useful for a certain mac version and above. Below a certain version I think there are issues with the library. I think the version is 14, but I can't remember off the top of my head.

This means two sets of wheels have to be made. One set for version > 10.X that bundles openblas and one set for version >14.x that doesn't. numpy is already doing this, but start from a lower macOS version.

rgommers · 2024-04-16T05:36:28Z

Yes indeed, we should start shipping extra wheels built against Accelerate for macOS >=14.0 for the next release, just like is done for numpy 2.0.0. They will be smaller and faster.

rgommers · 2024-06-14T15:41:28Z

This may be a good place to record that macOS 15 will include another update to Accelerate - it upgrade to LAPACK 3.11.0 with no change in symbol names. The extra LAPACK APIs will be available if the macOS deployment target is >= 15.0.

thalassemia requested review from andyfaff, ev-br, ilayn, larsoner, person142, peterbell10, rgommers, steppi and tylerjereddy as code owners January 6, 2024 03:35

thalassemia changed the title ~~Accelerate~~ ENH: Add Accelerate support for macOS 13.3+ Jan 6, 2024

thalassemia changed the title ~~ENH: Add Accelerate support for macOS 13.3+~~ BLD: Add Accelerate support for macOS 13.3+ Jan 6, 2024

lucascolley removed scipy.special scipy.io scipy.sparse.linalg scipy.linalg scipy.sparse scipy.optimize labels Jan 6, 2024

rgommers reviewed Apr 13, 2024

View reviewed changes

scipy/spatial/meson.build Outdated Show resolved Hide resolved

rgommers reviewed Apr 13, 2024

View reviewed changes

scipy/meson.build Show resolved Hide resolved