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
Remove dependency on openlibm #42299
base: master
Are you sure you want to change the base?
Conversation
|
How do I prevent the system image building step from trying to look for a libm? |
|
SpecialFunctions.jl needs this - that's the main big package, without which everything that depends on it will fail. But we will have it depend directly on |
|
|
Looks like tests are failing because this is still pulling in OpenLibm_jll as a stdlib |
|
Linux32 seems to have a ton of issues when using the system libm. So we do need to remove all calls to libm (#26434) if we want to get rid of this dependency. Luckily it's just a couple of functions, and only a couple of hundred lines of code. Of course, there's no urgency - and this PR shows what needs to happen when we are ready to excise. |
|
Is the failure of the |
|
Yeah, I think it is. My guess is that |
|
I thought LLVM will find the system libm. Do we link LLVM to openlibm? |
|
It builds fine on linux for me, but not on my mac My suspicion is that this has something to do with llvm not finding libm and perhaps a |
|
There is no |
|
Right - but I don't know where the linking to libm is happening and I need to intervene for mac. |
|
I think if you link with |
|
But wouldn't that do the right thing on Mac and find libSystem? |
|
One of the last steps here is to remove |
|
https://build.julialang.org/#/builders/90/builds/4890/steps/8/logs/stdio |
No - elsewhere in packages that use |
|
@nanosoldier |
|
The |
|
Your package evaluation job has completed - possible new issues were detected. A full report can be found here. |
We are using And MS recommends linking against to ref: tgamma, tgammaf, tgammal | Microsoft Docs julia> ccall((:tgamma, "api-ms-win-crt-math-l1-1-0.dll"), Float64, (Float64,), 10.0)
362880.00000000006 |
Would this also be why we have the |
I'm using Win10 x64. So, I checked two dlls.
But Then I found out that MSVC just calls // <math.h> => <corecrt_math.h>
// C:\Program Files (x86)\Windows Kits\10\Include\10.0.19041.0\ucrt\corecrt_math.h
// line 687
_Check_return_ __inline float __CRTDECL fmodf(_In_ float _X, _In_ float _Y)
{
return (float)fmod(_X, _Y);
}So maybe we just need to wrap the Bonus: If you do compile an x86 program that calls |
|
So, what is the right list of files to dlopen on Windows so that we get all the things openlibm has? Seems the list is slightly different for win64 and win32. Where in the codebase should we be doing this? On win32, the fmodf is being called from llvm generated code - so this is something that needs to be fixed in a way that llvm can call fmodf (can't just be a julia side fix - the issue is only in the codegen). |
|
somethings just didn't exist in win32, though minor ones, we had been implicitly forwarding those to openlibm |
Do we have a list? Can you point to where in the codebase? |
Further examination is needed.
I checked the repo for julia and openlibm. Maybe we don't have one.
The name of the macro |
|
So, stuff we forward implicitly to openlibm, can we not forward to the Julia versions (can we mark them |
|
Bump. The issue has all ticked (and SpecialFunctions works?). If this is about win32, then it's a tier2 platform. Can this be merged? |
|
Worth considering, but this will mean that we can never regain win32 without more work. There is not much to gain, but only to lose. @DilumAluthge Thoughts? I would be ok to merge this if we go all the way and drop win32 altogether from CI. |
|
Not sure what the issue was here exactly but I implemented a native fmod, which seemed to be a holdout? #47501 |
|
IIRC @vtjnash would like to promote Windows i686 to Tier 1, so that would block us from merging this PR. |
|
Oh wait, does Gabriel's PR mean that this PR won't break Windows i686? |
|
I'm not sure. It would mean that we don't need that specific intrinsic anymore. Which might make this work |
|
@gbaraldi You probably want to add your fmod to this PR, and see how it works out. |


Fix #26434