Use openblas/clapack on windows static#46904
Conversation
|
This simplifies the blas support table a lot as well, as now only on macos the apple framework is used. No changes on the ports individual port side was neccesary |
|
New table to update #24327 (review) if this gets merged:
(Code for easy copy-paste for others:) |
|
The blas/lapack monster strikes again. The last time which backends we choose was litigated was back in #24327 (see very similar table I wrote then: #24327 (review) ) where @Neumann-A wrote:
Can you explain if something has changed about this situation and why we aren't going to immediately get another PR trying to put things back? |
|
(I believe the intent was to use |
Nothing has changed, this is still the same situation. The real issue is that, because we use gfortran (or just, any mingw fortran compiler, as microsoft doesn't have one), we cannot link lapack-reference statically to anything on windows non-mingw. This is also printed by vcpkg (somewhere hidden in the long output), when trying to use lapack-reference on windows-static. So nothing has really changed, but windows-static has just always been "broken", working but not statically. Clapack might be a little slower, I don't know as I cannot find much online, however the ability to propperly support windows-static seems like a much better alternative. Since clapack is also just a port of lapack, the difference should not be as major as with other lapack implementations. On top of that lapack mentions that using the builtin blas implementation is very slow, so using However I am not a performance geek wrt lapack/blas, I am merely trying to get a static binary on windows. If you need some performance numbers on lapack vs clapack, I can try and get them, but it would likely be on linux. |
|
Ok there seems to be a bit of an issue. arpack-ng: Options: disable this for windows-static, or allow add a force-reference-cblas option coin-or-ipopt: This seems to be a a general incompatibility with openblas, as it also doesn't build on x64-linux, this just seems to be a case of not supporting openblas and requirering the reference blas implementation. Again, we can disable this for windows-static as it is a bad package, or add a force-reference-blas flag openimageio: No idea, seems to work locally (on windows-static). Weird that it can't find f2c, as it should be present. Also doesn't depend on lapack, not sure why it was build in CI or gives an issue related to lapack. openmvs/paraview: Seems to be the same issue as openimageio, havn't verified that it builds locally yet |
I would like to note that
I have difficulties with the fact the default LAPACK with openblas isn't the lapack from openblas... |
d2d2438 to
0ed290e
Compare
|
clapack is outdated. |
5d5dca1 to
c2d68ab
Compare
|
I created llvm/llvm-project#153618 so that LLVM might package flang with the next release on windows. So that vcpkg can use flang instead of gfortran in the future. |
|
I agree that this would be a much, much better solution, I am fine with keeping my fork alive for private use until vcpkg switches over to flang. If that is not an option, I would still suggest this PR gets merged (preferably disabling the broken ports on windows-static) as currently blas/lapack on windows-static is just broken |
|
@ras0219-msft @vicroms and I discussed today. I wrote this to frame the conversation:
@ras0219-msft asked "how does arpack-ng:arm64-windows work now?" answer: PS C:\Dev\vcpkg> .\vcpkg.exe install arpack-ng:arm64-windows --dry-run
Computing installation plan...
vcpkg-gfortran is only supported on 'windows & !arm & !uwp', which does not match arm64-windows. This usually means that there are known build failures, or runtime problems, when building other platforms. To ignore this and attempt to build vcpkg-gfortran anyway, rerun vcpkg with `--allow-unsupported`.@JAicewizard Please go ahead and make the change for windows-static as originally proposed and add entries to |
6771499 to
353a693
Compare
353a693 to
0ac6d41
Compare
|
That is much appreciated! I think I fixed all the issues related to this PR. I have no idea why |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Disabled |
| vcpkg-ci-opencv:x64-windows=pass | ||
| vcpkg-ci-opencv:x86-windows=pass | ||
| vcpkg-ci-openimageio:x64-windows-release=pass | ||
| vcpkg-ci-openimageio:x64-windows-static-md=pass |
There was a problem hiding this comment.
This passed for me locally. Are we sure it needs to be disabled?
There was a problem hiding this comment.
I will rerun with these enabled again, but if I disabled them it must have been broken in one way or another, I think it was some kind of cascade issue
|
Something in 1ES Hosted Pools for our non-Windows pool is broken :(. Will keep you posted. |
| @@ -731,6 +735,8 @@ omniorb:x64-android=fail | |||
| # opencc/deps/marisa-0.2.5/lib/marisa/grimoire/io/mapper.cc currently doesn't support UWP. | |||
| opencc:x64-android=fail | |||
| openimageio:arm64-windows-static-md=fail | |||
There was a problem hiding this comment.
I rechecked the ones below and they all passed for me. (arpack-ng and coin-or-ipopt are indeed broken)
PS C:\Dev\vcpkg> .\vcpkg.exe install --overlay-ports .\scripts\test_ports arpack-ng:x64-windows-static-md arpack-ng:x64-windows-static coin-or-ipopt:x64-windows-static-md coin-or-ipopt:x64-windows-static openmvs:x64-windows-static-md openmvs:x64-windows-static paraview:x64-windows-static-md paraview:x64-windows-static --keep-going
[...]
RESULTS
openmvs:x64-windows-static: SUCCEEDED: 0 ns
openmvs:x64-windows-static-md: SUCCEEDED: 0 ns
paraview:x64-windows-static: SUCCEEDED: 0 ns
paraview:x64-windows-static-md: SUCCEEDED: 0 ns
arpack-ng:x64-windows-static: BUILD_FAILED: 9 s
arpack-ng:x64-windows-static-md: BUILD_FAILED: 6.2 s
coin-or-ipopt:x64-windows-static: BUILD_FAILED: 43 s
coin-or-ipopt:x64-windows-static-md: BUILD_FAILED: 42 s
SUMMARY FOR x64-windows-static
SUCCEEDED: 2
BUILD_FAILED: 2
SUMMARY FOR x64-windows-static-md
SUCCEEDED: 2
BUILD_FAILED: 2There was a problem hiding this comment.
I removed the fails, lmk if its good. Some of these were still failing in CI, but it seems untrustworthy
|
FTR drive-by fix to clapack link lib export in #47060. |
Hopefully fixed now! But lab will be busy catching up.... |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
BillyONeal
left a comment
There was a problem hiding this comment.
Sorry for the delay! Thanks.
|
No worries! I understand this is a complex PR to merge, and I was slow myself as well. Thank you for merging this!! it is much appreciated |
Fixes #46715
./vcpkg x-add-version --alland committing the result.