openblas: cross-compilation support #24171
Conversation
This comment has been minimized.
This comment has been minimized.
Conan v1 pipeline ✔️All green in build 2 (
Conan v2 pipeline ✔️
All green in build 2 (
|
|
I was looking for this :) |
| } | ||
| options_description = { | ||
| "build_lapack": "Build LAPACK and LAPACKE", | ||
| "build_relapack": "Build with ReLAPACK (recursive implementation of several LAPACK functions on top of standard LAPACK)", | ||
| "use_thread": "Enable threads support", | ||
| "use_locking": "Use locks even in single-threaded builds to make them callable from multiple threads", | ||
| "dynamic_arch": "Include support for multiple CPU targets, with automatic selection at runtime (x86/x86_64, aarch64 or ppc only)", | ||
| "target": "OpenBLAS TARGET variable (see TargetList.txt)", |
There was a problem hiding this comment.
Where is TargetList.txt?
There was a problem hiding this comment.
It's part of OpenBLAS itself: https://github.com/OpenMathLib/OpenBLAS/blob/4e9144b39faa6b0542e1b7c183e0dab1a8637a07/TargetList.txt
| target = conan_arch_to_openblas_target.get(str(self.settings.arch)) | ||
| if target: | ||
| self.output.warning(f'Setting OpenBLAS TARGET={target} based on settings.arch. This may result in suboptimal performance. Set the "{self.name}/*:target=XXX" option to silence this warning.') | ||
| self.options.target = target |
There was a problem hiding this comment.
question here!
if we only set self.options.target when cross_building, that means that given a host profile, this recipe will have different package IDs depending on whether it was crossbuilt or not. This is not ideal - one may expect a "producer" such as CI to be able to cross-build, and a consumer on the "native" target system will not find that package because when not-crossbuild this option will default to None - unless I'm mistaken.
I wonder what values are mapped when NOT cross building - but if this file https://github.com/OpenMathLib/OpenBLAS/blob/442dec28df3c04b32e6f8630b40080d6a5715843/cmake/prebuild.cmake#L1402 is any indication, I have a suspicion that it is dynamically setting based on the features of the CPU of the machine doing the build? Then we're also in the same situation: for the same package ID, the contents may be different depending on which machine the build ran on, correct?
If this is the case - would it make sense to always give self.options.target a value derived from the arch setting - regardless of whether or not it is cross-compiled, and point the user to set one that better suits their needs if necessary? I think that solves that particular concern
There was a problem hiding this comment.
Yes, I agree. The reason why I set target=None by default is to keep the existing behavior and not break things for current users. But as you said, this is a lie, since OpenBLAS will dynamically change the options in that case, and this should result in different package IDs.
To complicate matters even further, there doesn't seem to be a way to disable the automatic detection in OpenBLAS, the logic you linked to is always executed if (NOT CMAKE_CROSSCOMPILING), so we'd need to patch that (and hope nothing else breaks) if we want to explicitly set TARGET from Conan.
And then there's DYNAMIC_ARCH, which includes multiple versions of each function, and dispatches dynamically based on the target it's being executed on. I haven't tried this myself, and it does of course increase binary size and compile times.
Does Conan have any standard way to deal with different microarchitectures? I suspect that any library that uses SIMD or other modern instructions runs into this specific issue, so maybe this should be standardized (e.g. as a setting in the user's profile). E.g. Spack supports this: https://spack.readthedocs.io/en/latest/basic_usage.html#support-for-specific-microarchitectures. An alternative could be to adopt the System V ABI micro-architecture levels with coarser granularity: https://gitlab.com/x86-psABIs/x86-64-ABI
A quick google search only yields some old discussions without solutions like conan-io/conan#847. This one about OpenBLAS's TARGET option is definitely relevant, though: #13478
There was a problem hiding this comment.
thanks @tttapa - indeed those are the issues.
I think the current PR is fine:
- it keeps the current behaviour
- it gives an ability that wasn't there before
while I think it's worth fixing this issues, since doing so may result in changing the default option value - it's probably best to handle that in a follow-up PR
|
Maybe it’s worth considering creating an MR on Conan? |
Specify library name and version: openblas/*
This PR adds support for cross-compiling OpenBLAS. In previous versions of the recipe, cross-building was explicitly disabled.
When cross-compiling, OpenBLAS requires the
TARGEToption to be set, so this option has also been added to the Conan recipe. By default, it is set toNonewhich has no effect. When cross-compiling, the defaultTARGETis determined based onsettings.arch.Native builds should not be affected by these changes.
Differences compared to other open pull requests for the OpenBLAS recipe:
Updated openblas from validate to validate_build #24038:
Checking for cross-compilation support should be done in
validate_build(), notvalidate().This change is included in the present PR as well, but the check is now different.
Openblas cross compile #23207:
Support for ARMv8 is also covered by the present PR, without the need for backporting patches.
#23207usestc.cache_variables["TARGET"] = str(self.settings.arch).upper(), which only works for some architectures, the dictionary in the present PR supports all common architectures, not just ARM.