shibatch / sleef

SIMD Library for Evaluating Elementary Functions, vectorized libm and DFT
https://sleef.org
Boost Software License 1.0
628 stars 126 forks source link

Allow build dft on MinGW #540

Closed Andarwinux closed 2 months ago

Andarwinux commented 3 months ago

It actually works fine.

Andarwinux commented 3 months ago

I don't know about gcc, but at least llvm-mingw works fine, I used sleefdft for rubberband's fft library and got better performance than fftw.

blapie commented 3 months ago

Hello! We are a bit concerned about the absence of testing for this one. But I suppose DFT can still be disabled manually. Any chance you could test with gcc?

blapie commented 3 months ago

Btw thanks for testing and reporting this, very useful! I'll try to test locally.

Andarwinux commented 3 months ago

Hello! We are a bit concerned about the absence of testing for this one. But I suppose DFT can still be disabled manually. Any chance you could test with gcc?

I'll try to add a MSYS2 CI that can test gcc and clang. but expect some things to not work. MSYS2 also support clang arm64 and I've done a preliminary evaluation of arm64 build with my cross build toolchain, but that's hard to do in the CI since GHA doesn't support Windows Arm64 runner.

Andarwinux commented 3 months ago

Unfortunately, the mingw-w64 gcc does not work at all. But clang passes the test, will add msys2 native build & test and llvm-mingw aarch64 & x86_64 cross build later.

blapie commented 3 months ago

I'll try to add a MSYS2 CI that can test gcc and clang. but expect some things to not work.

Yes, it is most likely why we were holding it for now. But we would be ok to start with only a subset of the tests.

MSYS2 also support clang arm64 and I've done a preliminary evaluation of arm64 build with my cross build toolchain, but that's hard to do in the CI since GHA doesn't support Windows Arm64 runner.

Yes, we have also done some work to enable Windows on Arm (for sleef/libm) that is not yet upstream, happy to try and merge that soon if that can help. However as you said it is not available on GH-hosted runners, so we will have to keep testing internally for now. Another issue with Windows on Arm is that libm tests rely heavily on POSIX, which is an issue with MSYS2. That's a shame because MSYS2 is quite a convenient environment to use. I have not checked if it's also the case for sleef/dft though, but I suspect it is.

blapie commented 3 months ago

Thanks for the contribution, impressive GHA skills!

I like the idea of the build/test-msys2 jobs, we were definitely missing testing for Windows (x86).

On the other hand, we did not desperately need a precommit workflow for llvm-mingw, but it is quite handy to be able to (cross-)compile for windows on both x86 and arm64, so happy try and merge that too.

Before getting into the details, would you mind moving both jobs to separate yaml files (even if it means duplicating code for build-native)? We might want to schedule and parametrise the 3 workflows differently. {build,test}-msys2 to build-and-test-windows.yml and build-llvm-mingw to build-cross-windows.yml?

Btw if you are aware of simple/nice ways to re-use bits of workflows (say build-native) in other workflows, please let us know.

Andarwinux commented 2 months ago

On the other hand, we did not desperately need a precommit workflow for llvm-mingw, but it is quite handy to be able to (cross-)compile for windows on both x86 and arm64, so happy try and merge that too.

Most MinGW users are cross-compile on Linux because the build are an order of magnitude faster, so I'd prefer cross-compile to be a first-class citizen.

Before getting into the details, would you mind moving both jobs to separate yaml files (even if it means duplicating code for build-native)? We might want to schedule and parametrise the 3 workflows differently. {build,test}-msys2 to build-and-test-windows.yml and build-llvm-mingw to build-cross-windows.yml?

Done. Native build are usually very fast, and since there is no longer need to pass native build files via artifacts, this is actually faster.

Btw if you are aware of simple/nice ways to re-use bits of workflows (say build-native) in other workflows, please let us know.

One possible way to do this is to save native builds to action cache, which is quite a bit faster than artifacts and can across workflows. But I suspect this would still be slower than build native and cross within same instance.

blapie commented 2 months ago

One possible way to do this is to save native builds to action cache, which is quite a bit faster than artifacts and can across workflows. But I suspect this would still be slower than build native and cross within same instance.

Thanks for the insight, action cache might be useful for other jobs. I like the present solution though, hopefully in the future a native build isn't even needed to cross-compile.

blapie commented 2 months ago

Regarding the libm and quad tests on msys2, would you be able to link us to the build or test failures? The main reason I expect the build to fail is that wait.h is not available on msys2, since it is POSIX. For this reason I think the suggested changes to CMakeLists are acceptable, until we find a workaround.

Do you have suggestions on ways to port a code that relies on wait.h (wait, fork, ...) to mingw/msys2?

@shibatch Have you ever considered ways to make the libm tester compatible with mingw?

shibatch commented 2 months ago

Since tester1 does not need to be compiled with mingw, the test should be possible by combining tester1 for Cygwin or linux, although it may be a bit troublesome to write the script for the test.

Another idea could be to add a new tester using tlfloat, which I am currently developing. With tlfloat, testing is possible even if libmpfr is not available.

blapie commented 2 months ago

Since tester1 does not need to be compiled with mingw, the test should be possible by combining tester1 for Cygwin or linux, although it may be a bit troublesome to write the script for the test.

Fair, it's true that they can be compiled independently, but I'm afraid this will confuse things more than they already are on Windows.

Another idea could be to add a new tester using tlfloat, which I am currently developing. With tlfloat, testing is possible even if libmpfr is not available.

That could be interesting! Looks like you already have support for several OS-es there too. Mpfr is not the worst dependency to have, but always good to remove dependencies I suppose.

Would tlfloat resolve the dependency on wait.h and POSIX in general? Is it able to handle multithreading too? My suggestion would have been to just brutally remove multithreading/calls to wait.h in tester1 on windows, but I'm not sure how to do that.

Alternatively, we could look at straight windows port for wait.h https://github.com/win32ports/sys_wait_h ?

shibatch commented 2 months ago

The new tester can be made pipe-free and multi-threading capable.

blapie commented 2 months ago

The new tester can be made pipe-free and multi-threading capable.

I suggest we create an issue specifically for this?

shibatch commented 2 months ago

Writing an Issue is fine, but there are other things to consider regarding this. First is how to link tlfloat from sleef.

If we implement the fp16 support mentioned here, it could be related to that as well. https://github.com/shibatch/sleef/issues/320

Andarwinux commented 2 months ago

Made some fixes and cleanups, and enabled LTO for clang.

Andarwinux commented 2 months ago

Rename yml to msys2 and llvm-mingw to avoid conflicts with future msvc/llvm-msvc{msvc-style, gnu-style}/llvm-msvc-cross{msvc-style, gnu-style}.

blapie commented 2 months ago

Are you happy for me to merge?

Andarwinux commented 2 months ago

Yes, LGTM.