Skip to content

Add some new musl math files to libc-rt library #15497

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
merged 1 commit into from
Nov 12, 2021
Merged

Add some new musl math files to libc-rt library #15497

merged 1 commit into from
Nov 12, 2021

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Nov 11, 2021

We had some reports of missing symbols in LTO builds after
the musl upgrade (#13006):
https://groups.google.com/g/emscripten-discuss/c/g_vkRRSCPUI/m/aT6fnUAtCQAJ?utm_medium=email&utm_source=footer

This change moves some of the new math-related source files
into the libc-mt library which is excluded from LTO (see comments
in in system_libs.py for background on this).

Verified that lto2.test_float_builtinsTest now passes with this
change but fails without (after musl upgrade).

This change fixes asan.test_float_builtins that is currently
failing. See #15498.

We have had reports of missing symbols in LTO builds after
the musl upgrade (#13006):
https://groups.google.com/g/emscripten-discuss/c/g_vkRRSCPUI/m/aT6fnUAtCQAJ?utm_medium=email&utm_source=footer

Also `lto2.test_float_builtinsTest` started failing on the
emscripten-releases FYI waterfall.

This change moves some of the new math-related source files
into the libc-mt library which is excluded from LTO (see comments
in in system_libs.py for background on this).

Fixes: #15506
@sbc100
Copy link
Collaborator Author

sbc100 commented Nov 12, 2021

Enabling auto-merge. I"m going to be OOO for a few hours but hopefully this can get landed ASAP so folks who are testing tot can get unblocked.

@sbc100 sbc100 enabled auto-merge (squash) November 12, 2021 15:24
@floooh
Copy link
Collaborator

floooh commented Nov 12, 2021

Would be nice to get the following files into the PR before it's merged:

__math_invalid.c, __math_invalidf.c', __math_invalidl.c

See:

#15506 (comment)

@sbc100 sbc100 merged commit 6160323 into main Nov 12, 2021
@sbc100 sbc100 deleted the fix_math_lto branch November 12, 2021 16:18
@floooh
Copy link
Collaborator

floooh commented Nov 12, 2021

Looks like the last 3 files didn't make it? I'll raise the issue again when the tot SDK has been updated :)

@sbc100
Copy link
Collaborator Author

sbc100 commented Nov 12, 2021

Oops, sorry, will open another PR

sbc100 added a commit that referenced this pull request Nov 12, 2021
sbc100 added a commit that referenced this pull request Nov 12, 2021
@floooh
Copy link
Collaborator

floooh commented Nov 16, 2021

Everything working now on my side. Thanks!

mmarczell-graphisoft pushed a commit to GRAPHISOFT/emscripten that referenced this pull request Jan 5, 2022
We have had reports of missing symbols in LTO builds after
the musl upgrade (emscripten-core#13006):
https://groups.google.com/g/emscripten-discuss/c/g_vkRRSCPUI/m/aT6fnUAtCQAJ?utm_medium=email&utm_source=footer

Also `lto2.test_float_builtinsTest` started failing on the
emscripten-releases FYI waterfall.

This change moves some of the new math-related source files
into the libc-mt library which is excluded from LTO (see comments
in in system_libs.py for background on this).

Fixes: emscripten-core#15506
mmarczell-graphisoft pushed a commit to GRAPHISOFT/emscripten that referenced this pull request Jan 5, 2022
sbc100 added a commit that referenced this pull request Sep 8, 2023
Continuation of #15497

Fixes: #19781
sbc100 added a commit that referenced this pull request Sep 8, 2023
Continuation of #15497

Fixes: #19781
sbc100 added a commit that referenced this pull request Sep 8, 2023
Continuation of #15497

Fixes: #19781
sbc100 added a commit that referenced this pull request Sep 8, 2023
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