Skip to content

Indirectly update to libm 0.2 #335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 27, 2023
Merged

Conversation

eclipseo
Copy link
Contributor

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

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: rust-lang#294
@eclipseo
Copy link
Contributor Author

eclipseo commented Sep 17, 2021

Let's try another way!

This is heavily inspired by cuviper patch here: dimforge/alga#93

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

@workingjubilee
Copy link
Member

I will review this patch when CI has been fixed.

@alexanderkjall
Copy link

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
Copy link
Member

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
Copy link
Member

Thank you.

@workingjubilee workingjubilee merged commit 2ffcc96 into rust-lang:master Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

packed_simd seems incompatible with libm 0.2
3 participants