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

Remove openlibm #26434

Open
10 of 17 tasks
simonbyrne opened this issue Mar 13, 2018 · 49 comments
Open
10 of 17 tasks

Remove openlibm #26434

simonbyrne opened this issue Mar 13, 2018 · 49 comments
Labels
excision Removal of code from Base or the repository external dependencies Involves LLVM, OpenBLAS, or other linked libraries maths Mathematical functions

Comments

@simonbyrne
Copy link
Contributor

simonbyrne commented Mar 13, 2018

Thanks to @pkofod we're quite close, but there are a couple of places left:

SpecialFunctions.jl also calls a few. Note however that SpecialFunctions now uses OpenLibm_jll and no longer relies on Julia providing openlibm.

@vchuravy
Copy link
Member

Aren't we also convincing LLVM to call Openlibm instead of the system libm?

@simonbyrne
Copy link
Contributor Author

Are we? I remember talking about it, but did we ever make that happen?

@vchuravy
Copy link
Member

I can't find any reference to it in the source code so we are probably not doing that and I just remember the same discussion.
We could probably resolve the libcalls llvm will lower them to similar to what I am trying to do in #26381

@simonbyrne
Copy link
Contributor Author

simonbyrne commented Mar 13, 2018 via email

@Keno
Copy link
Member

Keno commented Mar 13, 2018

Ideally we should figure out a generic mechanism to have LLVM request a function be constant folded with the implementation we have available in the compiler.

@vchuravy
Copy link
Member

vchuravy commented Mar 13, 2018 via email

@ararslan ararslan added maths Mathematical functions external dependencies Involves LLVM, OpenBLAS, or other linked libraries labels Mar 13, 2018
@pkofod
Copy link
Contributor

pkofod commented Mar 13, 2018

I have code for log2, log10, lgamma and remainder as well. I think _ldexp_exp as well, but it is very short either way. I just havn't really prioritized making new PRs given that the existing ones are moving rather slowly, and I have some deadlines outside of OSS.

@ararslan
Copy link
Member

@pkofod If you get the chance I recommend just opening PRs with your changes, and if you need to be away for a while, someone who knows what they're doing can push to the branch with any necessary changes if applicable.

@giordano
Copy link
Contributor

Regarding fastmath point, I have this PR still waiting for review: #24031. If it's good, I can add the missing functions

@ViralBShah
Copy link
Member

@pkofod Would you be able to push through your PRs?

Things like SpecialFunctions should be easy to update, and the fastmath could just fallback to the system libm initially perhaps?

@pkofod
Copy link
Contributor

pkofod commented Mar 27, 2018

What do you mean? If I can pr the remaining, or if the existing can be be finished soon ?

@ViralBShah
Copy link
Member

I was actually referring to bumping the existing ones if someone needs to review or do something about those. How far do you think we are from excising the remaining ones?

@StefanKarpinski
Copy link
Member

The above checklist with no check marks makes it look like none of this is done which I'm fairly sure it not the case at this point. @pkofod, would you mind updating the checkmarks here? Should be very satisfying since everyone knows that getting to check the boxes is why people make check lists.

@pkofod
Copy link
Contributor

pkofod commented May 15, 2018

Well, I think it might actually be quite accurate, as this was made after most of my prs were merged. The three open PRs are quite done as far as I remember, but I'm just waiting for feedback. I guess they have to be rebased by now, but I'd be happy to help get them merged asap to the extent that someone agrees to review. I'm on a quite tight time budget these months, so I can really only justify spending time on something that has a high chance of moving forward.

edit: Maybe I should clarify something. I know the potential people who could review the PRs are also busy, so this was not supposed to be me pointing fingers at anyone.

@ViralBShah
Copy link
Member

Let's tag folks who could help review and move things along. Perhaps @simonbyrne ?

@pkofod
Copy link
Contributor

pkofod commented May 15, 2018

I’ll rebase the existing PRs so we have something to work off of

edit: they're rebased. I hope tests go through, and if they do, I suggest we do nanosoldier runs on them.

@pkofod
Copy link
Contributor

pkofod commented May 16, 2018

The thing I need help with the most is if people agree that the test sets are sufficient (coverage and correctness).

@pkofod
Copy link
Contributor

pkofod commented May 17, 2018

I can't edit @simonbyrne 's post, but #27137 covers log2 and log10

@ViralBShah
Copy link
Member

It would be nice to get the outstanding PRs merged. It seems that we are almost ready to do so in many cases.

@einzigsue
Copy link

I have code for log2, log10, lgamma and remainder as well. I think _ldexp_exp as well, but it is very short either way. I just havn't really prioritized making new PRs given that the existing ones are moving rather slowly, and I have some deadlines outside of OSS.

I found these two lines in the file base/special/hyperbolic.jl
_ldexp_exp(x::Float64, i::T) where T = ccall(("__ldexp_exp", libm), Float64, (Float64, T), x, i)
_ldexp_exp(x::Float32, i::T) where T = ccall(("__ldexp_expf",libm), Float32, (Float32, T), x, i)
but neither gnu libm.so.6 nor intel/2018.2.199 libimf.so has the functions __ldexp_exp and __ldexp_expf defined.

Does it mean v1.0.0 has to be built with openlibm?

@ararslan
Copy link
Member

Correct. Julia's build system builds and bundles openlibm. It's largely invisible to the user.

@einzigsue
Copy link

then it might be better to disable USE_INTEL_LIBM from Make.inc

@ararslan
Copy link
Member

Sorry, I misread. No it doesn't have to be built with openlibm, but we strongly encourage it. Every USE_SYSTEM_* flag for Make is effectively "use at your own risk."

@einzigsue
Copy link

I thought USE_INTEL_LIBM is to configure intel tools.

@pkofod
Copy link
Contributor

pkofod commented Sep 26, 2018

@einzigsue ah yes I can see why that would be an issue if you try to swap out the const libm with some other libm that otherwise have all the functions we are calling using ccall. That’ll be a priority to get out.

@pkofod
Copy link
Contributor

pkofod commented Sep 30, 2019

__ldexp_exp can be crossed off the list

@oscardssmith
Copy link
Member

I believe your summary is about right. _ldexp_exp no longer exists in Base though. I believe the old exp implementations needed it, but it was removed from 1.6. rem and modf should be pretty easy to port. fma will be harder. I believe that this list is currently missing pow, which I am planning on replacing soon.

@ViralBShah
Copy link
Member

fma only uses openlibm on 32-bit I thought.

@oscardssmith
Copy link
Member

What about arm architectures that don't have native fma instructions?

@ViralBShah
Copy link
Member

Doesn't llvm do something there, like fallback to the non-fma version or something? I mean having openlibm does not solve that problem anyways, right?

@oscardssmith
Copy link
Member

I'm not the right person to ask here. I've done pretty much all my work assuming 64 bit x86...

@ViralBShah
Copy link
Member

I'll submit a PR that removes openlibm, and we'll see where it goes.

@ViralBShah
Copy link
Member

As seen in #42299 Linux32 seems to have a lot of failures when not using openlibm. It seems that we should move remainder, modff and fma to avoid openlibm before we can excise it.

@KristofferC
Copy link
Member

KristofferC commented Sep 30, 2021

The fma bug that is linked to #9847 was fixed in 2014: https://sourceware.org/bugzilla/show_bug.cgi?id=16064. I wonder if it is still relevant.

@ViralBShah
Copy link
Member

ViralBShah commented Oct 16, 2021

fma is the last issue to remove openlibm from base. #42299 (comment)

Can we count on llvm to provide the right version always? @vchuravy perhaps may know.

@oscardssmith
Copy link
Member

I'll take a look at a native Julia version by next weekend. If we want Julia to do it, we do need to figure out how to do the versioning.

@ararslan
Copy link
Member

I started on a native implementation of fma based on msun but haven't got it finished, cleaned up, and/or properly tested. Don't let that stop you though, Oscar, if you wanted to have a go at it.

@simonbyrne
Copy link
Contributor Author

Or we could tell people to use a glibc that is newer than 6 years old?

@StefanKarpinski
Copy link
Member

Can we emulate FMA with compensated arithmetic?

@StefanKarpinski
Copy link
Member

[He says, then realizing he has no idea how to do compensated multiplication]

@simonbyrne
Copy link
Contributor Author

Yes, but FMAs are notoriously difficult to get right for all the edge cases. A simpler idea is to just call the fma function, and manually reset the rounding mode each time (#42961): since this is just to deal with very old 32-bit linux installations, it seems like the simplest option.

simonbyrne added a commit that referenced this issue Nov 5, 2021
@KristofferC
Copy link
Member

I thought we already merged FMA though: #42783?

@oscardssmith
Copy link
Member

yeah. it is a total pain to get right, but we now have it in Base.

@simonbyrne
Copy link
Contributor Author

Oh, or we could do that...

Keno added a commit that referenced this issue Jan 19, 2022
This is more of a "Do we want to move in this direction RFC". As mentioned in #43786,
we currently have three implementations of these intrinsics:

1. The code generated by LLVM for the intrinsic
2. The code LLVM uses for constant folding the intrinsic
3. Our own runtime intrinsic used by the interpreter

This basically removes the third one, which will be required if we want to do
something about #26434 because we just forward these to libm. Of course we'll
still have to do something to teach LLVM how to constant fold these in a manner
compatible with what will actually end up running, but that's a separate issue.
Keno added a commit that referenced this issue Jan 20, 2022
This is more of a "Do we want to move in this direction RFC". As mentioned in #43786,
we currently have three implementations of these intrinsics:

1. The code generated by LLVM for the intrinsic
2. The code LLVM uses for constant folding the intrinsic
3. Our own runtime intrinsic used by the interpreter

This basically removes the third one, which will be required if we want to do
something about #26434 because we just forward these to libm. Of course we'll
still have to do something to teach LLVM how to constant fold these in a manner
compatible with what will actually end up running, but that's a separate issue.
@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Aug 30, 2022

fma is the last issue to remove openlibm from base. #42299 (comment)

I thought we already merged FMA though: #42783?

Can't we just remove openlibm, and ditch those users, without fma hardware (or force to use LTS or 1.8 for a little longer)? Or use the emulated one? [Or if it matters copy those two C functions from libm?]

FMA3 is supported in AMD processors starting with the Piledriver architecture and Intel starting with Haswell processors and Broadwell processors since 2014.

I think it may be actually CPUs older than June 2013, so 9+ years old.

What about arm architectures that don't have native fma instructions?

https://developer.arm.com/documentation/dui0491/g/Compiler-specific-Features/Fused-Multiply-Add--FMA--intrinsics

They are only supported for the Cortex-A5 and Cortex-M4 processors.

ARM Cortex-A5 with fma was announced in 2009.

EDIT:

fma turns out to just be badly broken on windows without hardware fma, and @oscardssmith will be posting a detailed issue around it. It was also always broken on 1.6.3.

That seems to be an argument for not using (its) libm, rather dropping those users (or copy those C libm functions, as stated above, from Linux libm, should work on Windows?).

I went looking if Python supports fma, and found it planned for 3.7, issue still open, they link to Go implementation and have one in Python too:
smasher164/fma@4e85d23

It has been hand-optimized to benefit from branch prediction, and
outperforms MUSL by ~5ns for RNE.

https://bugs.python.org/issue29282
https://bugs.python.org/file46304/fma_reference.py

The IEEE 754-2008 specification leaves one FMA corner case to the
implementation: namely whether the cases zero * infinity + nan and
infinity * zero + nan return nan or raise an invalid operation FPE.

By default, this implementation follows Intel's x64 FMA3 implementation,
which returns NaN in these cases. If corner_case_raises is True,
then this corner case raise ValueError instead

@StefanKarpinski
Copy link
Member

It kind of looks like Base Julia doesn't need to depend on OpenLibm anymore? At least based on the checkmark list at the top. Is that the case now? (@oscardssmith, @simonbyrne)

@oscardssmith
Copy link
Member

See #42299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
excision Removal of code from Base or the repository external dependencies Involves LLVM, OpenBLAS, or other linked libraries maths Mathematical functions
Projects
None yet
Development

Successfully merging a pull request may close this issue.