Skip to content

libc: replace musl's and MinGW's trigonometric functions with compiler_rt's #24034

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

Merged

Conversation

papparapa
Copy link
Contributor

@papparapa papparapa commented May 31, 2025

Part of #2879.
According to the instruction, this PR removes some musl files, including the following functions:

  • musl
    • sin
    • sinf
    • cos
    • cosf
    • sincos
    • sincosf
    • tan
    • tanf
  • MinGW (added after review)
    • sinf
    • cosf
    • sincos
    • sincosf
    • tanf

I confirmed that running zig build test-modules and zig-libc-test raised no errors during the trigonometric function tests.
zig-libc-test revealed some discrepancies between the results of long double trigonometric functions in compiler_rt and musl. Therefore, this PR is restricted to float and double functions.

- sin
- sinf
- cos
- cosf
- sincos
- sincosf
- tan
- tanf
@alexrp alexrp self-assigned this May 31, 2025
@alexrp
Copy link
Member

alexrp commented May 31, 2025

It seems like we also have

  • cosf
  • sincos (includes sincosl?)
  • sincosf
  • sinf
  • tanf

in lib/libc/mingw.

@papparapa
Copy link
Contributor Author

When deleting musl files, I was able to run the deletion after confirming with zig-libc-test that the function behavior did not change.
However, I am not sure what policy I can use to delete MinGW files.
The behavior of compiler_rt's sin function and musl's sin function are consistent, but MinGW's sin function may not be.
Do I need to make sure that the behavior of compiler_rt's sin function and MinGW's sin function match?
Or, even if the behavior does not match, can we assume that musl's sin function (compiler_rt's sin function) behavior is more appropriate and remove MinGW's sin function?

@alexrp
Copy link
Member

alexrp commented Jun 1, 2025

Do I need to make sure that the behavior of compiler_rt's sin function and MinGW's sin function match?

I don't think so. musl tends to be pretty good about math functions, so until we have evidence to the contrary, let's assume that it's a good baseline for math functions for all static libcs.

@papparapa
Copy link
Contributor Author

papparapa commented Jun 1, 2025

Thanks, I deleted the MinGW files with relief. (71ff383)
In lib/libc/mingw/math/arm/sincos.S and lib/libc/mingw/math/arm64/sincos.S, I removed only the sincos symbol.
It will not be too late to remove MinGW's sincosl at the same time as musl.

@papparapa papparapa changed the title libc: replace musl's trigonometric functions with compiler_rt's libc: replace musl's and MinGW's trigonometric functions with compiler_rt's Jun 1, 2025
@andrewrk andrewrk merged commit 597dd32 into ziglang:master Jun 3, 2025
9 checks passed
@andrewrk
Copy link
Member

andrewrk commented Jun 3, 2025

Nice, thank you

@papparapa papparapa deleted the remove-musl-trigonometric-function branch June 3, 2025 12:05
@alexrp alexrp removed their assignment Jun 3, 2025
@andrewrk
Copy link
Member

andrewrk commented Jun 3, 2025

Sorry @alexrp, I didn't notice you had assigned yourself or I would have left it in your hands.

@alexrp
Copy link
Member

alexrp commented Jun 3, 2025

No worries, I'm not calling dibs or anything. 🙂

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.

3 participants