Skip to content

Conversation

@maleadt
Copy link
Member

@maleadt maleadt commented Nov 17, 2020

@maleadt
Copy link
Member Author

maleadt commented Nov 17, 2020

Kicked-off a build with USE_BINARYBUILDER_OPENLIBM=false DEPS_GIT=openlibm here: https://build.julialang.org/#/builders/47/builds/1

@maleadt
Copy link
Member Author

maleadt commented Nov 17, 2020

Fails to build on Windows because we don't seem to have floating-point functions there. Presumable this used to work, https://github.com/JuliaLang/julia/pull/36588/files#diff-b93461e172b1a09513b591004c100b10a2a3fcf407d51fc19e6be1cb73c246d4, because we were linking openlibm before.

@vtjnash You mentioned, #38427 (comment), we should have functions like fenv available on Windows, however ccall(:fegetenv, Cint, (Ptr{Cvoid},), C_NULL) (and the build here) fails. Do you have any insights?

Checking the libraries that are part of x86_64-w64-mingw32, listing only the ones that have something fesetenv-like:

/usr/x86_64-w64-mingw32/sys-root/mingw/lib/libmingwex.a
lib64_libmingwex_a-fesetenv.o:
0000000000000000 T fesetenv
                 U fesetenv

/usr/x86_64-w64-mingw32/sys-root/mingw/lib/libmsvcr120.a
0000000000000000 I __imp_fesetenv
0000000000000000 T fesetenv

/usr/x86_64-w64-mingw32/sys-root/mingw/lib/libmsvcr120_app.a
0000000000000000 I __imp_fesetenv
0000000000000000 T fesetenv

/usr/x86_64-w64-mingw32/sys-root/mingw/lib/libmsvcr120d.a
0000000000000000 I __imp_fesetenv
0000000000000000 T fesetenv

/usr/x86_64-w64-mingw32/sys-root/mingw/lib/libucrt.a
0000000000000000 I __imp_fesetenv

/usr/x86_64-w64-mingw32/sys-root/mingw/lib/libucrtapp.a
0000000000000000 I __imp_fesetenv

/usr/x86_64-w64-mingw32/sys-root/mingw/lib/libucrtbase.a
0000000000000000 I __imp_fesetenv

@vtjnash
Copy link
Member

vtjnash commented Nov 17, 2020

It looks like a .a library, so you need to force link it.

@maleadt
Copy link
Member Author

maleadt commented Nov 17, 2020

I guess these functions are only in the ucrt, and not the msvcrt.dll we're using:

julia> crt = Libdl.dlopen("msvcrt.dll")
Ptr{Nothing} @0x00007ffbb0920000

julia> Libdl.dlsym(crt, "ferror")
Ptr{Nothing} @0x00007ffbb096cad0

julia> Libdl.dlsym(crt, "fegetenv")
ERROR: could not load symbol "fegetenv":
The specified procedure could not be found.
julia> crt = Libdl.dlopen("ucrtbase.dll")
Ptr{Nothing} @0x00007ffbaee70000

julia> Libdl.dlsym(crt, "fegetenv")
Ptr{Nothing} @0x00007ffbaee768d0

Switching to the ucrt seems quite an large change. Alternatively, we could ship libmingwex and use that?

@maleadt
Copy link
Member Author

maleadt commented Nov 17, 2020

It looks like a .a library, so you need to force link it.

No, that results in lots of multiple definition errors. Looks like mingwex gets automatically linked already, presumably for C99 support when using msvcrt. So I'm trying to get it to export the fegetround/fesetround defs from that static library, is there a clean way to do that other than introducing a 'fake' use somewhere in julia or libjulia?

Also, any thoughts about this:

Tim@DESKTOP-1NUTSID ~/julia
$ nm -g usr/bin/libjulia.dll | grep feget
0000000065471370 T fegetround

Tim@DESKTOP-1NUTSID ~/julia
$ ./julia.bat -q
julia> using Libdl

julia> lib = Libdl.dlopen("usr/bin/libjulia.dll")
Ptr{Nothing} @0x00000000028f0000

julia> Libdl.dlsym(lib, "fegetround")
ERROR: could not load symbol "fegetround":
The specified procedure could not be found.

@vtjnash
Copy link
Member

vtjnash commented Nov 17, 2020

--require-defined=fesetround

Though to re-export it, I think you may need to mark it during the build, as exported symbols have a different name mangling

@maleadt
Copy link
Member Author

maleadt commented Nov 17, 2020

Thanks, I'll try that. Alternatively, we do an if Sys.iswindows() for these calls and have them use openlibm there, but that would require some reordering of the bootstrap includes.

@maleadt maleadt force-pushed the tb/unexport_fenv branch 2 times, most recently from d8dfd81 to e9b31cb Compare November 17, 2020 17:38
@maleadt
Copy link
Member Author

maleadt commented Nov 17, 2020

Couldn't get it to work; nm and objdump were lying to/confusing me as the symbols weren't visible in Dependencies.
Anyway, decided to take the simpler option of introducing jl_fegetround/jl_fesetround.
Kicked off another build to use openlibm from master here: https://build.julialang.org/#/builders/47/builds/2

@vtjnash
Copy link
Member

vtjnash commented Nov 17, 2020

Could also just add another special case to the jitlayers file. I know we already have a few there.

@maleadt maleadt marked this pull request as ready for review November 18, 2020 11:41
@maleadt
Copy link
Member Author

maleadt commented Nov 18, 2020

Could also just add another special case to the jitlayers file.

Ah, yes. This seems like a simpler solution though, and does the job as well.

CI failures unrelated (happen on master too), and the manual build using openlibm#master, https://build.julialang.org/#/buildersj/47/builds/2, successfully builds and tests on all platforms too. Let's generate new binaries now.

@nico202
Copy link
Contributor

nico202 commented Nov 18, 2020

I'm trying to build julia#110f1256ac with openlibm 0.7.3 and it's failing with

LoadError("sysimg.jl", 3, LoadErjuliaor("Base.jl", 269, LoadError("floatfuncs.jl", 345, ErrorException("could not load symbol \"fesetround\":\n/tmp/guix-build-julia-1.5.3-git.drv-0/source/usr/bin/../lib/libopenlibm.so: undefined symbol: fesetround"))))

Is this related?
Thanks

@maleadt
Copy link
Member Author

maleadt commented Nov 18, 2020

Yes, how come you are using openlibm 0.7.3 already? It needs this PR.

@staticfloat Looks like the x86_64-apple-darwin build has dropped its '14' tag. What do I need to change here, or in Yggdrasil?
EDIT: I renamed the asset 🙈

@nico202
Copy link
Contributor

nico202 commented Nov 18, 2020

I'm preparing the next package for guix, just saw the new tagged openlibm release and updated it (we use USE_SYSTEM_OPENLIBM=1).
I'll try with 0.7.2 in the meanwhile, thanks

@maleadt maleadt added external dependencies Involves LLVM, OpenBLAS, or other linked libraries maths Mathematical functions labels Nov 18, 2020
@maleadt
Copy link
Member Author

maleadt commented Nov 18, 2020

Should be good to go now.

@maleadt maleadt merged commit 18c759f into master Nov 19, 2020
@maleadt maleadt deleted the tb/unexport_fenv branch November 19, 2020 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external dependencies Involves LLVM, OpenBLAS, or other linked libraries maths Mathematical functions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openlibm fenv changes broke CUDA

4 participants