[naga]: Add no_std polyfill for round_ties_even for f32 and f64
#7585
Conversation
bushrat011899
changed the title
no_std polyfill for round_ties_even for f32 and f64no_std polyfill for round_ties_even for f32 and f64
|
This PR will actually fix |
bushrat011899
force-pushed
the
no_std-naga
branch
from
226490c to
861d264
Compare
| # Check with all features except "std". | ||
| cargo clippy --target ${{ matrix.target }} ${{ matrix.extra-flags }} -p wgpu-types --no-default-features --features strict_asserts,fragile-send-sync-non-atomic-wasm,serde,counters | ||
| cargo clippy --target ${{ matrix.target }} ${{ matrix.extra-flags }} -p naga --no-default-features --features dot-out,compact |
There was a problem hiding this comment.
This comment isn't correct for naga - I think you should pull them out into their own lines instead of attaching them.
There was a problem hiding this comment.
I've just updated the comments to be more broad. I do like the two groups (no features, all compatible features), but I'm happy to split it up (either crate by crate or just each line on it's own)
naga/src/proc/constant_evaluator.rs
Outdated
| /// <https://github.com/rust-lang/rust/issues/96710>. | ||
| /// | ||
| /// [polyfill source]: https://github.com/imeka/ndarray-ndimage/blob/8b14b4d6ecfbc96a8a052f802e342a7049c68d8f/src/lib.rs#L98 | ||
| fn round_ties_even<T: num_traits::float::FloatCore>(x: T) -> T { |
There was a problem hiding this comment.
I am really worried about the precision of this. wgsl requires perfect rounding. When it was only used for f16 (using f64 internals) it had a ton of excess precision. But using f64 on f64 I really worry about this.
I think it would be better to take a dep on libm and call rint/rintf which use a slightly different algorithm.
There was a problem hiding this comment.
Perfectly reasonable. I was unaware libm actually offered an option for round_ties_even (but TBF the libm documentation leaves a lot to be desired). I've reverted my changes and just switched to libm::rint/f
bushrat011899
force-pushed
the
no_std-naga
branch
from
73bad8d to
fdca21d
Compare
Update comments around `no_std` CI task
bushrat011899
force-pushed
the
no_std-naga
branch
from
fdca21d to
f7ed1ba
Compare
Connections
Description
round_ties_evenf16polyfill forf32andf64usingnum_traits::float::FloatCore, specifically forno_stdsupportnagato the Checkno_stdCI action now that it is MVP compatibleTesting
Squash or Rebase?
Squash
Checklist
cargo fmt.RunN/Ataplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.Notes
f32::round_ties_evenorf64::round_ties_even, since I don't believe there is substantial value added with such a feature.f16polyfill. However, there are substantial tests for this exact method in this num-traits PR I opened. This PR would just allow us to continue working onno_stdsupport while we wait for that PR to be accepted (or not).