rust-lang / packed_simd

Portable Packed SIMD Vectors for Rust standard library
https://rust-lang.github.io/packed_simd/packed_simd_2/
Apache License 2.0
589 stars 74 forks source link

Indirectly update to libm 0.2 #335

Closed eclipseo closed 1 year ago

eclipseo commented 2 years ago

Unfortunately, libm 0.2 remove the F32Ext and F64Ext extension traits, which makes it harder to choose the different methods. However, this was already dealt with in rust-num/num-traits#144 for Float, so we can piggy-back on that here, no longer using libm directly.

Close: #294

eclipseo commented 2 years ago

Let's try another way!

This is heavily inspired by cuviper patch here: https://github.com/dimforge/alga/pull/93

Tested on multiple arches here: https://koji.fedoraproject.org/koji/taskinfo?taskID=75866870 (with tests disabled for arm and i686).

workingjubilee commented 2 years ago

I will review this patch when CI has been fixed.

alexanderkjall commented 1 year ago

Is there a reason for not merging this?

If so, wouldn't it be possible to use libm 0.2 directly like this:

diff --git a/src/codegen/math/float/tanh.rs b/src/codegen/math/float/tanh.rs
index 2c0dd3d..630e2b6 100644
--- a/src/codegen/math/float/tanh.rs
+++ b/src/codegen/math/float/tanh.rs
@@ -15,18 +15,18 @@ macro_rules! define_tanh {
             use core::intrinsics::transmute;
             let mut buf: [$basetype; $lanes] = unsafe { transmute(x) };
             for elem in &mut buf {
-                *elem = <$basetype as $trait>::tanh(*elem);
+                *elem = $trait(*elem);
             }
             unsafe { transmute(buf) }
         }
     };

     (f32 => $name:ident, $type:ty, $lanes:expr) => {
-        define_tanh!($name, f32, $type, $lanes, libm::F32Ext);
+        define_tanh!($name, f32, $type, $lanes, libm::tanhf);
     };

     (f64 => $name:ident, $type:ty, $lanes:expr) => {
-        define_tanh!($name, f64, $type, $lanes, libm::F64Ext);
+        define_tanh!($name, f64, $type, $lanes, libm::tanh);
     };
 }

@@ -43,11 +43,11 @@ define_tanh!(f64 => tanh_v4f64, f64x4, 4);
 define_tanh!(f64 => tanh_v8f64, f64x8, 8);

 fn tanh_f32(x: f32) -> f32 {
-    libm::F32Ext::tanh(x)
+    libm::tanhf(x)
 }

 fn tanh_f64(x: f64) -> f64 {
-    libm::F64Ext::tanh(x)
+    libm::tanh(x)
 }

 gen_unary_impl_table!(Tanh, tanh);
workingjubilee commented 1 year ago

The answer is simply that I found the thought of writing up an entire new CI workflow exhausting, so I didn't do it, and then someone was kind enough to break ground on it. The CI still isn't fixed, but it is much closer, and I did some followup work to make it almost-functional. It's mostly missing the Android targets.

workingjubilee commented 1 year ago

Thank you.