Repository navigation
Use core::intrinsics::powi{f32,f64} for powi instead of num-traits - #666
Merged
Merged
Conversation
mikwielgus
requested review from
Firestar99,
LegNeato and
eddyb
as code owners
October 6, 2026 23:56
mikwielgus
force-pushed
the
core-intrinsics-powi
branch
from
October 6, 2026 23:57
9a73de7 to
a26809b
Compare
This undoes 8eecf61 code changes, except for the `powi.rs` test, which is retained. To provide `powi` in a no-`std` environment, instead of pulling it from the `num-traits` crate and defining a custom intrinsic for it, we just define `powi` methods on `f32` and `f64` directly. In these methods, we just emit `core::intrinsics::powif32` and `core::intrinsics::powif64`, respectively. We can do this thanks to the `#[rustc_allow_incoherent_impl]` internal rustc attribute, which allows us to add inherent `impl` blocks on types defined in other crates, which would otherwise violate coherence rules. (This trait is similarly used inside the Rust compiler to have `core` module provide some alternative implementations of `std`'s methods in a no-`std` environment.) In a normal Rust library, the use of such an internal rustc attribute would have been unacceptable, but since Rust-GPU is a compiler, it seems reasonable. Unlike `f32` and `f64`, there is no need to implement `powi` for `f16` and `f128` because these already have `powi` methods correctly implemented in `core` (see https://doc.rust-lang.org/core/primitive.f16.html#method.powi and https://doc.rust-lang.org/core/primitive.f128.html#method.powi). And `f32` and `f64` actually will have these too, once Rust issue #137578 (rust-lang/rust#137578) is resolved, rendering this commit obsolete. But noone knows how many years it will take them to get that going, so let's just fix the problem on Rust-GPU's end. There are other mathematical methods, e.g. `powf`, `sqrt`, and many others, that Rust-GPU still needs `num-traits` for. I will be moving their implementations to Rust-GPU and removing `num-traits` altogether in subsequent commits, after and if this PR is accepted -- this is just the first step to see if this solution is acceptable at all. I ran `cargo test`, `cargo compiletest`, `cargo difftest`. Tests pass. `powi.rs` test's output changed, but only marginally -- I have blessed it. Reference: Rust-GPU#520
mikwielgus
force-pushed
the
core-intrinsics-powi
branch
from
October 7, 2026 00:09
a26809b to
37df3bf
Compare
Collaborator
|
Neat, I did not know about |
Firestar99
approved these changes
Oct 7, 2026
Firestar99
left a comment
Member
There was a problem hiding this comment.
Thanks for the patch, looks good from my side!
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
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.
This undoes 8eecf61 code changes, except for the
powi.rstest, which is retained.To provide
powiin a no-stdenvironment, instead of pulling it from thenum-traitscrate and defining a custom intrinsic for it, we just definepowimethods onf32andf64directly. In these methods, we just emitcore::intrinsics::powif32andcore::intrinsics::powif64, respectively.We can do this thanks to the
#[rustc_allow_incoherent_impl]internal rustc attribute, which allows us to add inherentimplblocks on types defined in other crates, which would otherwise violate coherence rules.(This trait is similarly used inside the Rust compiler to have
coremodule provide some alternative implementations ofstd's methods in a no-stdenvironment.)In a normal Rust library, the use of such an internal rustc attribute would have been unacceptable, but since Rust-GPU is a compiler, it seems reasonable.
Unlike
f32andf64, there is no need to implementpowiforf16andf128because these already havepowimethods correctly implemented incore(see https://doc.rust-lang.org/core/primitive.f16.html#method.powi and https://doc.rust-lang.org/core/primitive.f128.html#method.powi). Andf32andf64actually will have these too, once Rust issue #137578 (rust-lang/rust#137578) is resolved, rendering this commit obsolete. But noone knows how many years it will take them to get that going, so let's just fix the problem on Rust-GPU's end.There are other mathematical methods, e.g.
powf,sqrt, and many others, that Rust-GPU still needsnum-traitsfor. I will be moving their implementations to Rust-GPU and removingnum-traitsaltogether in subsequent commits, after and if this PR is accepted -- this is just the first step to see if this solution is acceptable at all.I ran
cargo test,cargo compiletest,cargo difftest. Tests pass.powi.rstest's output changed, but only marginally -- I have blessed it.Reference: #520