Skip to content
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

On Windows only build library with major soversion #97

Merged

Conversation

giordano
Copy link
Collaborator

@giordano giordano commented Dec 29, 2022

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 giordano force-pushed the mg/one-library-to-rule-them-all branch from c5c94f3 to e305c13 Compare December 29, 2022 18:02
@giordano
Copy link
Collaborator Author

giordano commented Dec 29, 2022

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

@giordano giordano force-pushed the mg/one-library-to-rule-them-all branch 2 times, most recently from 835c13e to 86a97c3 Compare January 3, 2023 17:59
@giordano
Copy link
Collaborator Author

giordano commented Jan 3, 2023

@staticfloat do you have any clue of what's wrong with Windows? Note that this is necessary for JuliaLang/julia#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 giordano requested a review from staticfloat January 3, 2023 18:15
@giordano
Copy link
Collaborator Author

giordano commented Jan 4, 2023

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
Copy link
Collaborator Author

giordano commented Jan 5, 2023

@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
Copy link
Member

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 giordano force-pushed the mg/one-library-to-rule-them-all branch from 86a97c3 to fdcbd82 Compare January 5, 2023 20:52
@giordano
Copy link
Collaborator Author

giordano commented Jan 5, 2023

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
Copy link
Member

"-Wl,--trace" seems to not be supported by Windows linker. 😮‍💨

@giordano
Copy link
Collaborator Author

giordano commented Jan 5, 2023

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
Copy link
Member

amontoison commented Jan 5, 2023

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
Copy link
Collaborator Author

giordano commented Jan 5, 2023

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

@amontoison
Copy link
Member

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

@giordano
Copy link
Collaborator Author

giordano commented Jan 7, 2023

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 giordano force-pushed the mg/one-library-to-rule-them-all branch 3 times, most recently from 663dbe1 to 1b5d730 Compare January 7, 2023 10:37
@giordano giordano force-pushed the mg/one-library-to-rule-them-all branch from 1b5d730 to 88b6fa0 Compare January 7, 2023 10:38
@giordano giordano force-pushed the mg/one-library-to-rule-them-all branch from 88b6fa0 to 4bfd078 Compare January 7, 2023 10:45
@giordano
Copy link
Collaborator Author

giordano commented Jan 7, 2023

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

@giordano giordano linked an issue Jan 7, 2023 that may be closed by this pull request
@giordano
Copy link
Collaborator Author

giordano commented Jan 20, 2023

I tested in JuliaPackaging/Yggdrasil#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
Copy link
Collaborator Author

Oh, that's due to

ifeq ($(OS),WINNT)
@mkdir -p $(DESTDIR)$(prefix)/lib
@cp -a $(builddir)/$(LIB_MAJOR_VERSION).a $(DESTDIR)$(prefix)/lib
endif
@staticfloat Why do we do this at all? The copy looks unnecessary at best, if anything the file should be moved.

@staticfloat
Copy link
Member

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
Copy link
Collaborator Author

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 JuliaLang/julia#47676 and think about the duplicate later?

This avoids installing a duplicate import library on Windows.
@giordano giordano force-pushed the mg/one-library-to-rule-them-all branch from d786d60 to 6a61f91 Compare January 21, 2023 11:39
@giordano
Copy link
Collaborator Author

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

@staticfloat staticfloat merged commit d00e6ca into JuliaLinearAlgebra:main Jan 21, 2023
@giordano giordano deleted the mg/one-library-to-rule-them-all branch January 21, 2023 16:46
@giordano giordano linked an issue Jan 24, 2023 that may be closed by this pull request
@stahta01 stahta01 mentioned this pull request Feb 26, 2023
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.

Null pointers in LP64 on Windows Some problems with library on Windows
3 participants