Initial implementation of core_float_math
#138087
Merged
+4,711
−3,704
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
tgross35
force-pushed
the
core-float-math
branch
from
9cc8ffd to
b74488e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 6, 2025
Initial implementation of `core_float_math` Since [1], `compiler-builtins` makes a certain set of math symbols weakly available on all platforms. This means we can begin exposing some of the related functions in `core`, so begin this process here. It is not possible to provide inherent methods in both `core` and `std` while giving them different stability gates, so standalone functions are added instead. This provides a way to experiment with the functionality while unstable; once it is time to stabilize, they can be converted to inherent. For `f16` and `f128`, everything is unstable so we can move the inherent methods. The following are included to start: * floor * ceil * round * round_ties_even * trunc * fract * mul_add * div_euclid * rem_euclid * powi * sqrt * abs_sub * cbrt These mirror the set of functions that we have in `compiler-builtins` since [1]. Tracking issue: rust-lang#137578 [1]: rust-lang/compiler-builtins#763 r? `@ghost` try-job: aarch64-gnu try-job: arm-android tru-job: armhf-gnu try-job: test-various try-job: x86_64-apple-1 try-job: aarch64-apple try-job: i686-msvc-1 try-job: x86_64-msvc-ext2
This comment was marked as outdated.
This comment was marked as outdated.
tgross35
force-pushed
the
core-float-math
branch
from
b74488e to
12ad9b8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 6, 2025
Initial implementation of `core_float_math` Since [1], `compiler-builtins` makes a certain set of math symbols weakly available on all platforms. This means we can begin exposing some of the related functions in `core`, so begin this process here. It is not possible to provide inherent methods in both `core` and `std` while giving them different stability gates, so standalone functions are added instead. This provides a way to experiment with the functionality while unstable; once it is time to stabilize, they can be converted to inherent. For `f16` and `f128`, everything is unstable so we can move the inherent methods. The following are included to start: * floor * ceil * round * round_ties_even * trunc * fract * mul_add * div_euclid * rem_euclid * powi * sqrt * abs_sub * cbrt These mirror the set of functions that we have in `compiler-builtins` since [1]. Tracking issue: rust-lang#137578 [1]: rust-lang/compiler-builtins#763 r? `@ghost` try-job: aarch64-apple try-job: aarch64-gnu try-job: arm-android tru-job: armhf-gnu try-job: dist-various-1 try-job: dist-various-2 try-job: i686-msvc-1 try-job: test-various try-job: x86_64-apple-1 try-job: x86_64-msvc-ext2
This comment was marked as outdated.
This comment was marked as outdated.
tgross35
force-pushed
the
core-float-math
branch
from
12ad9b8 to
8d57e5a
Compare
This comment has been minimized.
This comment has been minimized.
tgross35
force-pushed
the
core-float-math
branch
from
8d57e5a to
eabddae
Compare
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 6, 2025
Initial implementation of `core_float_math` Since [1], `compiler-builtins` makes a certain set of math symbols weakly available on all platforms. This means we can begin exposing some of the related functions in `core`, so begin this process here. It is not possible to provide inherent methods in both `core` and `std` while giving them different stability gates, so standalone functions are added instead. This provides a way to experiment with the functionality while unstable; once it is time to stabilize, they can be converted to inherent. For `f16` and `f128`, everything is unstable so we can move the inherent methods. The following are included to start: * floor * ceil * round * round_ties_even * trunc * fract * mul_add * div_euclid * rem_euclid * powi * sqrt * abs_sub * cbrt These mirror the set of functions that we have in `compiler-builtins` since [1], with the exception of `powi` that has been there longer. Tracking issue: rust-lang#137578 [1]: rust-lang/compiler-builtins#763 r? `@ghost` try-job: aarch64-apple try-job: aarch64-gnu try-job: arm-android tru-job: armhf-gnu try-job: dist-various-1 try-job: dist-various-2 try-job: i686-msvc-1 try-job: test-various try-job: x86_64-apple-1 try-job: x86_64-msvc-ext2
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
bors
added
S-waiting-on-author
tgross35
force-pushed
the
core-float-math
branch
2 times, most recently
from
4375018 to
77794c6
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 6, 2025
Initial implementation of `core_float_math` Since [1], `compiler-builtins` makes a certain set of math symbols weakly available on all platforms. This means we can begin exposing some of the related functions in `core`, so begin this process here. It is not possible to provide inherent methods in both `core` and `std` while giving them different stability gates, so standalone functions are added instead. This provides a way to experiment with the functionality while unstable; once it is time to stabilize, they can be converted to inherent. For `f16` and `f128`, everything is unstable so we can move the inherent methods. The following are included to start: * floor * ceil * round * round_ties_even * trunc * fract * mul_add * div_euclid * rem_euclid * powi * sqrt * abs_sub * cbrt These mirror the set of functions that we have in `compiler-builtins` since [1], with the exception of `powi` that has been there longer. Tracking issue: rust-lang#137578 [1]: rust-lang/compiler-builtins#763 r? `@ghost` try-job: aarch64-apple try-job: aarch64-gnu try-job: arm-android tru-job: armhf-gnu try-job: dist-various-1 try-job: dist-various-2 try-job: i686-msvc-1 try-job: test-various try-job: x86_64-apple-1 try-job: x86_64-msvc-ext2
|
test-various passed, the others likely only need tweaks for f128 config. r? @Amanieu |
bors
added
the
S-waiting-on-author
The previous commit moved all test files from `std` to `core` so git understands the move. Not all functionality is actually testable in `core`, however, so perform move the relevant portions back. Changes from inherent to module methods is also done since this is the form of math operations available in `core` (as `core_float_math`).
Per [1], MinGW has an incorrect fma implementation. This showed up in tests run with cranelift after adding float math operations to `core`. Presumably we hadn't noticed this when running tests with LLVM because LLVM was constant folding the result away. Rust issue: rust-lang#140515 [1]: https://sourceforge.net/p/mingw-w64/bugs/848/
|
@bors try |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 14, 2025
Initial implementation of `core_float_math` Since [1], `compiler-builtins` makes a certain set of math symbols weakly available on all platforms. This means we can begin exposing some of the related functions in `core`, so begin this process here. It is not possible to provide inherent methods in both `core` and `std` while giving them different stability gates, so standalone functions are added instead. This provides a way to experiment with the functionality while unstable; once it is time to stabilize, they can be converted to inherent. For `f16` and `f128`, everything is unstable so we can move the inherent methods. The following are included to start: * floor * ceil * round * round_ties_even * trunc * fract * mul_add * div_euclid * rem_euclid * powi * sqrt * abs_sub * cbrt These mirror the set of functions that we have in `compiler-builtins` since [1], with the exception of `powi` that has been there longer. Details for each of the changes is in the commit messages. Tracking issue: rust-lang#137578 [1]: rust-lang/compiler-builtins#763 try-job: aarch64-gnu tru-job: armhf-gnu try-job: i686-msvc-1 try-job: test-various try-job: x86_64-mingw-1 try-job: x86_64-mingw-2
|
☀️ Try build successful - checks-actions |
|
Discussed in person @bors r=Amanieu |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing bf5a38d (parent) -> 777d372 (this PR) Test differencesShow 1962 test diffsStage 0
Stage 1
(and 504 additional test diffs) Additionally, 1358 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 777d372772aa3b39ba7273fcb8208a89f2ab0afd --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
This was referenced May 17, 2025
|
Finished benchmarking commit (777d372): comparison URL. Overall result: ❌✅ regressions and improvements - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (secondary -1.4%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary 0.6%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 774.349s -> 775.816s (0.19%) |
|
This landing in nightly has unfortunately broken Linebender's Name resolution seems to prefer these new functions over the ones defined on the I'm not sure how to determine if this class of breakage is classed as allowable. I think it would in theory be a regression on stable once 1.89 is stable. However, no matter where these functions live, it would be possible to adversarially trigger this for any unstable function being added. |
|
Are you glob importing something from std? This is something we do try to discourage because that means that std adding any new api can break things (rust-lang/rust-clippy#13961). Regardless of the outcome here, please consider fixing this in However, this is also intended to be temporary API that isn’t worth breaking anybody over, and |
|
For reference – fix for color: linebender/color#175 |
For reference, all the primitive types are re-exported in |
No, not that I'm aware of. We're only importing the (module) We have now made a remedial release of color (I don't know if this impacts prior minor versions - I'm still on mobile). |
|
It's hard to evaluate the breakage without the crater run, so we should either:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since 1,
compiler-builtinsmakes a certain set of math symbolsweakly available on all platforms. This means we can begin exposing some
of the related functions in
core, so begin this process here.It is not possible to provide inherent methods in both
coreandstdwhile giving them different stability gates, so standalone functions are
added instead. This provides a way to experiment with the functionality
while unstable; once it is time to stabilize, they can be converted to
inherent.
For
f16andf128, everything is unstable so we can move the inherentmethods.
The following are included to start:
These mirror the set of functions that we have in
compiler-builtinssince 1, with the exception of
powithat has been there longer.Details for each of the changes is in the commit messages.
Tracking issue: #137578
try-job: aarch64-gnu
tru-job: armhf-gnu
try-job: i686-msvc-1
try-job: test-various
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2