Fix or remove num_traits for performance

@LegNeato

We can't get rid of num_traits for silly rust reasons:
core::f32::powf is marked as #[rustc_allow_incoherent_impl], meaning it doesn't actually "exist" unless some other crate declares it. I'm not too familiar on the details, but it seems like when you have std it exists and if you only have core it's declared but doesn't exist and using it will fail with:

  error[E0599]: no function or associated item named `powf` found for type `f32` in the current scope
     --> crates/restir-shader/src/material/pbr/eval.rs:159:25
      |
  159 |     f0 + (1.0 - f0) * f32::powf(f32::clamp(1.0 - cos_theta, 0.0, 1.0), 5.0)
      |                            ^^^^ function or associated item not found in `f32`
      |
      = help: items from traits can only be used if the trait is in scope

see rust-lang/rust#149347 on this sillyness

The workaround has usually been this use statement:

#[cfg(target_arch = "spirv")]
use spirv_std::num_traits::Float;

num_traits with std feature will call std functions and with libm feature / in #[no_std] environments call the appropriate libm function. For SPIR-V, our spirv-std configures the feature for you and we then intercept those libm calls and replace them with intrinsics.

While researching this, I've noticed this has been discussed in 2021 on the embark repo already, and their conclusion was:

I think this is a fully external issue, then - rust-gpu is working "as intended", faithfully compiling the code that it is given, which doesn't use an intrinsic. The way to fix this, then, would be to change upstream code to be less branchy (e.g. requesting that libm adds a powi intrinsic, and having num-traits call it).

I don't mind if we want to merge this anyway, so it's not a trap to end users @LegNeato.

But num-traits isn't a perfect solution either: Technically, it doesn't actually reimplement all f32 functions. For example, it excludes euclid, for which you need to use the Euclid trait, which takes args by reference instead of by value, so you'll get awful code like this (which initially sparked my investigation into this last summer):

#[cfg(feature = "std")]
let rem = |x: f32| x.rem_euclid(1.);
#[cfg(not(feature = "std"))]
let rem = |x: f32| x.rem_euclid(&1.);

Originally posted by @Firestar99 in #518 (comment)

Is this issue about the general usability crap surrounding some functions being defined in core but not actually available, cause that has nothing to do with performance. Or is this about powi in particular performing badly?

The only reason we care is because the impl works but is not performant. If, for example, num_traits called out to spirv_std or used asm on spirv we wouldn't really care as much regardless of impl.

Hello. Regarding the powi performance problem here. Wouldn't it be solved by just upstreaming the following code to num-traits?:

#![cfg_attr(feature = "nightly", feature(core_intrinsics))]

//(...)

    #[cfg(feature = "nightly")]
    fn powi(self, n: i32) -> f32 {
        core::intrinsics::powif32(self, n)
    }

(while of course making the currently existing num-traits's powi implementation get skipped on nightly)

I can send a PR with this to num-traits, if you think this is a good idea. :)

That seems like a good idea to me, not sure it is appropriate for upstream though...up to the num_trait folks! Thanks for suggesting it 🤙

I started implementing this for num-traits, but then ran into this warning:

warning: the feature `core_intrinsics` is internal to the compiler or standard library
  --> src/lib.rs:20:42
   |
20 | #![cfg_attr(feature = "nightly", feature(core_intrinsics))]
   |                                          ^^^^^^^^^^^^^^^
   |
   = note: using it is strongly discouraged
   = note: `#[warn(internal_features)]` on by default

strongly discouraged is a stern warning, so I abstained -- a highly popular, public, conservative Rust library like num_traits is unlikely to want to have anything to do with something like that. So fixing num-traits is most likely out of question at least until Rust creates some more stable interface for powi.

However, since Rust-GPU is a compiler (and a nightly-only one, to add), I suppose that using an internal compiler feature is not going to be an issue here. So I will be instead looking into removing the num-traits dependency, replacing it with something that uses core::intrinsics::powif32 and core::intrinsics::powif64 on Rust-GPU project's side.

I suppose you're okay with this?

Yeah, we are trying to remove special casing but perhaps this is one we should just do as it is so widely used. I think we already have machinery in the project to target and replace a specific crate's code with our own. Thanks for looking into this!

Use `core::intrinsics::powi{f32,f64}` for `powi` instead of `num-traits`

Navigation Menu

Repository navigation

Fix or remove `num_traits` for performance #520

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Fix or remove num_traits for performance #520

Description

Activity

Firestar99 commented on Jan 23, 2026

LegNeato commented on Jan 25, 2026

mikwielgus commented on Oct 2, 2026

LegNeato commented on Oct 5, 2026

mikwielgus commented on Oct 5, 2026

LegNeato commented on Oct 5, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Fix or remove `num_traits` for performance #520