Skip to content

Skip extra linker flags on Windows ARM64 in get_pkg_config() #200

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

Merged
merged 2 commits into from
Jun 6, 2025

Conversation

Harishmcw
Copy link
Contributor

Hi @mattip and @matthew-brett ,

  • Upon setting up CI environment for building SciPy on WoA using the current scipy_openblas32 wheel, the build fails due to missing include libraries as seen in this log.

  • The root cause is due to the inclusion of extra linker flags (-lgfortran -lquadmath), which are only required for GNU-based compilers which are not required for WoA as it uses LLVM toolchain.

  • To fix this, the get_pkg_config() function in the init.py is updated to skip extra include directories when the architecture is Windows ARM64 which ensures clean builds on WoA without affecting x64 builds.

  • I updated the version in pyproject.toml and made sure it matches git describe --tags --abbrev=8 in OpenBLAS at the OPENBLAS_COMMIT

@mattip
Copy link
Collaborator

mattip commented Jun 5, 2025

I don't understand the platform difference. Why doesn't llvm need libfortran and libquadmath? Does the build of OpenBLAS conditionally use libfortran on mingw? And how did this work up to now: what happens on x86_64 if someone uses clang-cl (i.e. llvm) to build SciPy?

@rgommers
Copy link
Collaborator

rgommers commented Jun 6, 2025

@mattip those flags and runtime libraries are specific to gfortran, and the build for win_arm64 uses flang-new instead. So this should be correct.

I don't know exactly under which conditions flang-new requires runtime libraries, but it seems to work without with scipy-openblas32 and numpy. I unpacked a wheel (scipy_openblas32-0.3.29.265.0-py3-none-win_arm64.whl) from PyPI and checked, no LLVM runtime library vendored in:

$ cat scipy_openblas32-0.3.29.265.0.dist-info/RECORD
scipy_openblas32/__init__.py,sha256=_wDxG0WM1n4BXYXdFieOf5xaVqMe8t4ufpGTNySdvoM,4735
scipy_openblas32/__main__.py,sha256=9TkPSA3EuDFmVf4dX5sqj1sniC-SLZMhLkH9HZ2E2Uc,131
scipy_openblas32/include/cblas.h,sha256=TTtp1sMxWt6tK0Yelf40OY4HLGQCBhok4hS9nBdrXCY,57951
scipy_openblas32/include/f77blas.h,sha256=LZXknUsf01jPFxWG5TyrqcFaystjYjLjRLKv_aCA-KI,50661
scipy_openblas32/include/lapack.h,sha256=eLNxY8vKlVhzILz6WDAJ54UQCjkrZ5SeHg108lrwKBc,770885
scipy_openblas32/include/lapacke.h,sha256=91mJs1HyCd7rQefQ2VraJt8GJM73oGge31_QJhqw0Rg,866756
scipy_openblas32/include/lapacke_config.h,sha256=oZnrc2Mf6zl96xr5rGY0z105ajUzsrOCFRkNkcKu2ck,5631
scipy_openblas32/include/lapacke_mangling.h,sha256=EezUpb4dNuLg1hCRF-xRBGEAuB1LyQRmXs7uBBohqVE,491
scipy_openblas32/include/lapacke_utils.h,sha256=nEo47Vfdtp07_kKFIMWAOXfLiTEm4f10tVk0ZNlfJgE,35855
scipy_openblas32/include/openblas_config.h,sha256=z785JTZOv_QyxGURF75SByrbomUGbdJPQOj21YMpqp0,4663
scipy_openblas32/lib/scipy_openblas.dll,sha256=0G-_2wiISL5MnVGGgoXqCvuK_urdm1FlsThUR2Omr1k,10191360
scipy_openblas32/lib/scipy_openblas.lib,sha256=PhHXin7cFdWczdbFZQMn78Xbkyj3Hm1om5gUxrzAaJM,2016742
scipy_openblas32/lib/cmake/openblas/OpenBLASConfig.cmake,sha256=gF5ggITfKwrsEDu-B4HyQNad4O-Mge0tdcgZs1eqayo,3725
scipy_openblas32/lib/cmake/openblas/OpenBLASConfigVersion.cmake,sha256=ExkgdT7cmyjRKGCWZUeeUzzeh8YGgQm6QJQS7yJA5J0,1909
scipy_openblas32-0.3.29.265.0.dist-info/licenses/LICENSE.txt,sha256=-R1WxGEyUEJxuNYzlcks5_QLa5qZR_roWcoGqjeTra4,1344
scipy_openblas32-0.3.29.265.0.dist-info/METADATA,sha256=A9hCk1oi3NbSKOyp_QJah_v3eLhDa1CKWzgSrxRxBX4,3723
scipy_openblas32-0.3.29.265.0.dist-info/WHEEL,sha256=zaaOINJESkSfm_4HQVc5ssNzHCPXhJm0kEUakpsEHaU,91
scipy_openblas32-0.3.29.265.0.dist-info/top_level.txt,sha256=wt8IOfeY89VfU8u842dJaKChZD1EUFGrJKn2r4FaJjA,17
scipy_openblas32-0.3.29.265.0.dist-info/RECORD,,

@rgommers
Copy link
Collaborator

rgommers commented Jun 6, 2025

xref the discussion on scipy/scipy#21562 (comment) that motivated this PR.

Copy link
Collaborator

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Since OpenBLAS is built with flang-new, this looks correct to me and I think we should merge it and publish new wheels to progress on SciPy for win_arm64.

The use of flang-new seems unlikely to change I think, since there is no gfortran on WoA and flang-new is anyway more desirable and we expect it to mature further.

@Mugundanmcw
Copy link

I don't understand the platform difference. Why doesn't llvm need libfortran and libquadmath? Does the build of OpenBLAS conditionally use libfortran on mingw? And how did this work up to now: what happens on x86_64 if someone uses clang-cl (i.e. llvm) to build SciPy?

@mattip

  • MinGW toolchain is built on top of the GNU toolchain. So it uses Gfortran and GCC for openBLAS compilation.
  • Libquadmath is GCC's quad-precision math library that provides __float128 and __complex128 operations for GCC and GFortran compiler.
  • libgfortran is the static runtime library for programs compiled with the GNU Fortran compiler. It provides essential support for Fortran features like I/O, math functions, and array handling during static linking.
  • LLVM toolchain contain its own static library such as FortranRuntime & FortranDecimal which provides all above mentioned functionality. In such cases, addition of these GNU based libraries are incompatible and not required while using LLVM toolchain.
  • Currently on X64, the SciPy build is supported using either GCC+GFortran(RTools40-GNU Toolchain) or ifx (intel fortran compiler)+MSVC(Cl compiler), I could see from the CI that scipy_openblas32 is only used when the GNU toolchain is used for compilation and ifx+MSVC I could see the base venilla openblas is being utilized which confirms that scipy_openblas32 is used only when GNU toolchain is to build of SciPy on x64 Windows.
  • Alternatively, I tried building SciPy on X64 Windows with MSVC+ifx with prebuilt scipy_openblas32 wheel, but the build fails due to a linking issue of missing library "gfortran.lib", which confirms that scipy_openblas32 for X64 can be used only for compilation of SciPy using GNU-based toolchain. Please find the complete build logs, which I have attached below.

logs.txt

@mattip
Copy link
Collaborator

mattip commented Jun 6, 2025

Ahh, so the difference is that on all other platforms we use gcc/gfortran (with libgfortran and libquadmath) and for win-arm64 we use flang-new.

I am OK to merge this and release new wheels as-is, but I think the condition is misleading. It implies something is different with the runtime platform, when the root cause is the compiler used to build openblas and thus in the wheel itself. Maybe in a future PR we could refactor the logic to set libs_flags into the build here, much like the way we mangle the packgage name depending on the interface, and not have any misleading conditionals in get_pkg_config.

@rgommers
Copy link
Collaborator

rgommers commented Jun 6, 2025

It implies something is different with the runtime platform, when the root cause is the compiler used to build openblas and thus in the wheel itself.

I don't think there is any difference between these in practice. We do not support building scipy-openblas and scipy with different Fortran compilers.

@mattip mattip merged commit 8bb8a5e into MacPython:main Jun 6, 2025
19 checks passed
@mattip
Copy link
Collaborator

mattip commented Jun 6, 2025

I uploaded the scipy_openblas32-0.3.29.265.1 and scipy_openblas64-0.3.29.265.1 wheels with this fix to PyPI. Thanks @Harishmcw

@charris
Copy link
Collaborator

charris commented Jun 6, 2025

@mattip Will you be updating NumPy and should I wait for it to do a 2.3.0 release?

@mattip
Copy link
Collaborator

mattip commented Jun 6, 2025

I will only be available on Sunday. NumPy builds and passes tests without this, do we need it for f2py?

@rgommers
Copy link
Collaborator

rgommers commented Jun 6, 2025

No, there is no actual Fortran code in NumPy, so this won't matter - only for SciPy it does matter.

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.

5 participants