JuliaLinearAlgebra / libblastrampoline

Using PLT trampolines to provide a BLAS and LAPACK demuxing library.
MIT License
66 stars 17 forks source link

On Windows only build library with major soversion #97

Closed giordano closed 1 year ago

giordano commented 1 year ago

Keeping multiple copies of LBT on Windows is simply useless, wasteful, and error prone.

Refs

I haven't tested whether it actually builds, I'm hoping CI will tell me.

giordano commented 1 year ago

FreeBSD CI is broken because we have no nightlies for this platform. This is unrelated to this PR

giordano commented 1 year ago

@staticfloat do you have any clue of what's wrong with Windows? Note that this is necessary for https://github.com/JuliaLang/julia/pull/47676, so help with pushing this on Windows would be appreciated.

Side note, the Windows builds print tons of

make: Warning: File '../../src/Make.inc' has modification time 28740 s in the future
make: warning:  Clock skew detected.  Your build may be incomplete.

messages (28740 seconds == 7:59 hours, which is suspicously close to the difference between Pacific time and UTC).

giordano commented 1 year ago

I should note that this is technically an ABI breakage for Windows, but frankly the ABI of the Windows build has always been very inconsistent and kinda broken, so that making a new major version to signal it looks an overkill.

giordano commented 1 year ago

@amontoison this is failing the dgemmt test on Windows, any clue?

C:\workdir\test\dgemmt_test/dgemmt_test.c:67: undefined reference to `dgemmt_'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\workdir\test\dgemmt_test/dgemmt_test.c:68: undefined reference to `dgemmt_'
collect2.exe: error: ld returned 1 exit status
make: *** [Makefile:9: C:/Users/ContainerAdministrator/AppData/Local/Temp/jl_BS5Tst/dgemmt_test.exe] Error 1
┌ Error: compilation failed
│   srcdir = "C:\\workdir\\test\\dgemmt_test"
│   prefix = "C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\jl_BS5Tst"
│   cflags = "-g"
│   ldflags = "\"-LC:/Users/ContainerAdministrator/AppData/Local/Temp/jl_0iYeVs/output/bin\" \"-LC:/Users/ContainerAdministrator/.julia/artifacts/e32fbb6cb3c81caf8ba4aecdf54a6fc215996b78/bin\" \"-LC:/Users/ContainerAdministrator/.julia/artifacts/1c814d3529d610a4da8763928eb037b452e2a20c/bin\" \"-LC:/buildkite-agent/bin\" -lblastrampoline"
└ @ Main C:\workdir\test\runtests.jl:43
amontoison commented 1 year ago

Is it possible that Windows try to use another libblastrampoline.dll? dgemmt symbol was added recently (v5.3.0) and if we don't find the symbol, I highly suspect that we link with an older version.

giordano commented 1 year ago

I pushed a commit to make the linker more verbose. We'll see the output once we get an available runner, hopefully by tomorrow

amontoison commented 1 year ago

"-Wl,--trace" seems to not be supported by Windows linker. :face_exhaling:

giordano commented 1 year ago

It works with our mingw toolchain in BinaryBuilder:

sandbox:${WORKSPACE} # echo 'int main(){}' | cc -x c - -Wl,--trace
/opt/x86_64-w64-mingw32/bin/../lib/gcc/x86_64-w64-mingw32/4.8.5/../../../../x86_64-w64-mingw32/bin/ld: mode i386pep
/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/crt2.o
/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/crtbegin.o
/tmp/ccimOipk.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-tlssup.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-charmax.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-mingw_helpers.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-xtxtmode.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-_newmode.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-wildcard.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-natstart.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-crt_handler.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-cinitexe.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-dllargv.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-merr.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-usermatherr.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-pseudo-reloc.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-CRT_fp10.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-gccmain.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-gs_support.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-tlsmcrt.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-tlsthrd.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-pesect.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingw32.a)lib64_libmingw32_a-pseudo-reloc-list.o
(/opt/x86_64-w64-mingw32/bin/../lib/gcc/x86_64-w64-mingw32/4.8.5/libgcc.a)_chkstk_ms.o
(/opt/x86_64-w64-mingw32/bin/../lib/gcc/x86_64-w64-mingw32/4.8.5/libgcc.a)_ctors.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmingwex.a)lib64_libmingwex_a-mingw_matherr.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00082.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00055.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00096.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)lib64_libmsvcrt_os_a-__p__fmode.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00081.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)lib64_libmsvcrt_os_a-invalid_parameter_handler.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)lib64_libmsvcrt_os_a-__p__acmdln.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs01037.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs01096.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs01045.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00138.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00120.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00289.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00952.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00558.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00090.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs01075.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)lib64_libmsvcrt_os_a-acrt_iob_func.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00973.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00098.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00991.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs01129.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00926.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00939.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00980.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs01099.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhh.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00223.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00113.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqhs00083.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libmsvcrt.a)djeqht.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01409.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01393.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs00742.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01221.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01493.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01491.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs00629.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs00768.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs00552.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs00556.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs00798.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01130.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01222.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01229.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01236.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01458.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs00551.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01424.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs00318.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs01444.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs00983.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs00282.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhs00891.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqhh.o
(/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/libkernel32.a)dfeqht.o
/opt/x86_64-w64-mingw32/bin/../x86_64-w64-mingw32/sys-root/lib/../lib/crtend.o

I thought it'd work with mingw toolchain also on a real Windows system. What do you think would be the alternative, if this doesn't work?

amontoison commented 1 year ago

I don't understand why the error is thrown by Clang on Windows: https://buildkite.com/julialang/libblastrampoline/builds/72#018583b7-ca32-46f8-85a7-02949f1bcfe5/163-166 Maybe it could help you. I don't the equivalent flag for the Clang compiler, sorry Mosè.

giordano commented 1 year ago

That's macOS, not Windows, and Clang uses -t, not --trace, but I don't care about macOS/FreeBSD here.

amontoison commented 1 year ago

Sorry, I thought that it was a Windows build. Forget my two previous messages.

giordano commented 1 year ago

https://buildkite.com/julialang/libblastrampoline/builds/72#018583b7-c73b-4818-8224-1a44b112aa44/206-907

[ Info: Compiling `dgemmt_test` against blastrampoline (C:\Users\ContainerAdministrator\.julia\artifacts\1c814d3529d610a4da8763928eb037b452e2a20c\bin\mkl_rt.2.dll) in C:\Users\ContainerAdministrator\AppData\Local\Temp\jl_vmn1K6
make: Warning: File '../../src/Make.inc' has modification time 28742 s in the future
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../lib/crt2.o
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/crtbegin.o
C:\Users\ContainerAdministrator\AppData\Local\Temp\cckwUvVz.o
C:/buildkite-agent/bin/libblastrampoline.dll

I guess this is indeed linking against some old libblastrampoline.dll in PATH. I have no clue of what's in C:/buildkite-agent/bin/, but the file should be called libblastrampoline-5.dll, and be in a temporary directory.

giordano commented 1 year ago

This is finally working everywhere! Thanks for the tip about the old lbt being pulled in.

giordano commented 1 year ago

I tested in https://github.com/JuliaPackaging/Yggdrasil/pull/6122 that this now produces a single shared library, but I just realised that there are also two import libraries (they were there also before). Sigh.

giordano commented 1 year ago

Oh, that's due to https://github.com/JuliaLinearAlgebra/libblastrampoline/blob/1eb1fd3dde58c3003fc7a7b46940311ace8d8474/src/Makefile#L61-L64 @staticfloat Why do we do this at all? The copy looks unnecessary at best, if anything the file should be moved.

staticfloat commented 1 year ago

We're using cp because when you're doing make install, you don't want to get rid of the stuff in the build tree (otherwise you'd have to re-link). I'm installing the .a here because I thought that .a files typically go into lib, not bin, but . I think the bug here is that cp -a $(builddir)/libblastrampoline*$(SHLIB_EXT)* $(DESTDIR)$(prefix)/$(binlib)/ picks up the .dll.a file, but I'll be honest, I'm not 100% certain where the .dll.a file should go. Looking around online, I think it's still preferred to put it in lib and not bin, but I'm willing to be convinced otherwise if you feel strongly about it.

Regardless, we should have the import library in only one location, so if we still want it to be in lib, we should change the cp to a mv to move from the destination prefix's bin directory to the lib directory.

giordano commented 1 year ago

I think the bug here is that cp -a $(builddir)/libblastrampoline*$(SHLIB_EXT)* $(DESTDIR)$(prefix)/$(binlib)/ picks up the .dll.a file

Uhm, yeah, that's probably the main problem. I misread the line I linked in my previous post, I thought we were copying the file from bin to lib, instead is from the build directory to lib.

Anyway, this is unrelated to this PR, we had this duplication also before, should we merge this and tag a new release so that we can move on with https://github.com/JuliaLang/julia/pull/47676 and think about the duplicate later?

giordano commented 1 year ago

Ok, duplicate import library should have been fixed by the last commit, so this is good to go now.