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

Fix math libraries not being linked on some platforms #594

Merged
merged 3 commits into from
Apr 30, 2024

Conversation

Amjad50
Copy link
Contributor

@Amjad50 Amjad50 commented Apr 15, 2024

This is a continuation/fix of 018616e. In that commit, we made it add the math functions to all platforms (except apple-targets and windows), and use weak linking, so that it can be used if the system doesn't have those functions.

Didn't notice mod math was behind another set of cfg, so removed it as well here.

There is an issue with clippy, so disabled it on the file as a whole

@Amjad50 Amjad50 force-pushed the fix_libm_weak_link branch from 75eb6ec to 00baf68 Compare April 15, 2024 12:07
@Amanieu
Copy link
Member

Amanieu commented Apr 16, 2024

Rather than messing with clippy attributes, just disable it in CI for now.

Though this should be fixed properly by updating the libm crate.

@Amjad50
Copy link
Contributor Author

Amjad50 commented Apr 30, 2024

Updated libm

Amjad50 added 3 commits April 30, 2024 23:28
This is a continuation/fix of 018616e. In that commit, we made it add
the math functions to all platforms (except apple-targets and windows),
and use `weak` linking, so that it can be used if the system doesn't
have those functions.

Didn't notice `mod math` was behind another set of `cfg`, so removed it
as well here.
The solution is not pretty, but not sure why we still get clippy
warning from one of the files in `libm` even though we use
`allow(clippy::all)`
@Amanieu Amanieu force-pushed the fix_libm_weak_link branch from acee57a to dd85395 Compare April 30, 2024 21:28
@Amanieu Amanieu enabled auto-merge April 30, 2024 21:28
@ChrisDenton ChrisDenton disabled auto-merge April 30, 2024 21:30
@ChrisDenton ChrisDenton enabled auto-merge April 30, 2024 21:31
@ChrisDenton
Copy link
Member

Oops sorry for the noise. I accidentally tapped a button when scrolling on mobile. Hopefully I undid my mistake.

@ChrisDenton ChrisDenton merged commit 2cbde5b into rust-lang:master Apr 30, 2024
23 checks passed
tgross35 added a commit to tgross35/compiler-builtins that referenced this pull request Jun 18, 2024
In <rust-lang#594>, symbols
for the Rust port of libm were made always weakly available. This seems
to be causing problems on platforms with ABI issues, as explained at
<rust-lang/rust#125016 (comment)>.

Disable Rust libm on x86 without sse2 to mitigate this.
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